-
Notifications
You must be signed in to change notification settings - Fork 46
Add support for creating envs from requirements.txt files via pip #172
base: develop
Are you sure you want to change the base?
Add support for creating envs from requirements.txt files via pip #172
Conversation
…es via pip. I found myself frequently wishing this existed, so I added it. It will use conda packages if available, and fallback to pip otherwise. There was a bit of nonsense necessary to get editable packages working properly, but it all works.
I tweaked this a bit so that it now pulls in all dependencies of packages in requirements.txt and checks to see if those are available as conda packages. That way people don't end up with pip-installed numpy and other stuff like that. |
…e versions of pip.
All tests are now passing, so this is good for review. |
…all_pip_dependencies.
I have no idea why this build is failing on Travis when it works fine on my machine. |
""" | ||
pip_cmd = ('pip', 'install', '--src', temp_dir, '--download', temp_dir, | ||
'--no-use-wheel', '-r', requirements_path) | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment elsewhere. Please use isdir to check if dir exists, then create if necessary rather than try/except.
Sorry I don't have time to help you troubleshoot Travis right now. I would like to merge this if you can get it past Travis. Please see my other comments. This is a nice addition - thanks for your work on it. |
@dan-blanchard what's gonna happen if in the same directory you have an |
The
I can certainly do that if that's what y'all want me to, but I went with this since it is so similar to what is happening for the pip requirements in |
@dan-blanchard I think going with specs is a better approach because you'll take advantage of the architecture already in place for creating new environments. |
@@ -80,6 +80,8 @@ def execute(args, parser): | |||
# FIXME conda code currently requires args to have a name or prefix | |||
if args.name is None: | |||
args.name = env.name | |||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't fully understand what's going on here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this the environment won't get a name, because requirements.txt
files cannot contain the name of the environment.
I'm starting to feel like this might be outside the scope of the project. What does @msarahan thinks about? |
I don't think it's out of scope, necessarily. I see it as a good compatibility layer that may make conda a more compelling platform. The best platform isn't conda or pip, it's both. I don't fully understand the long-term support implications, though. It sort of joins conda to pip, and requires you to keep supporting pip once people start using this feature. What happens if/when pip is replaced down the line? You'll have to support pip likely longer than pip itself is supported. I don't think this issue is a deal breaker. I just think we need to discuss long-term implications, and figure out if any engineering solution can reduce future risk. |
@msarahan that's my concern. |
# properly. | ||
specs = list(chain(*[s.split() if '-e' in s else [s] for s in specs])) | ||
# Directory where pip will store VCS checkouts for editable packages | ||
src_dir = join(config.envs_dirs[0], env.name, 'src') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like config.envx_dirs[0]
is the location of the root environment.
how is this gonna work when we want to install into some other environment? (like the one we have just created)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.envs_dirs[0]
should be $CONDA_ROOT/envs
, and I'm joining that with the name of the environment and src
, so the final path will be $CONDA_ROOT/envs/$CONDA_ENV/src
. That's exactly what we want.
@githubfun, I've been successfully using my branch since September without any issues, so if this is something you need as much as I do, I suggest you just clone my branch and install it in your conda root environment with |
Any movement on adding this officially? I was talking to somebody at work about how they can use conda with requirements.txt files now and then I remembered it's only with my fork. |
One thing I haven't seen mentioned here, However using At the end of the day what I (and probably others) want is a consistent workflow across projects, whether they use a requirements.txt or environment.yml. Something like
Which is what this PR would let me do. So yeah +1 to this. |
@malev any word on how to get this merged in? what can we do to help? |
Yeah, the main advantage of the approach here is that this will support editable packages on GitHub and the like, which are currently not supported by |
hey @dan-blanchard, like you probably know, I'm really interested by this. Are you using this approach in production ? As of today, is there any other ways to achieve the same goal ? |
hey @mlaprise, we still aren't using conda in production at Parse.ly, but I use this at least once a week for development. I don't know of any other way to achieve the same goal. |
FWIW I would very much like to see this addition merged. I have wanted it so many times. |
@jni this repository has been deprecated, if there's still interested in this PR it should be re-opened in @ccordoba12 @kalefranz perhaps using the GH API to post a comment in all open issues and PRs to let people now that the developement has been moved to |
I found myself frequently wishing this existed, so I added it.
It will use conda packages if available, and fallback to pip otherwise.
There was a bit of nonsense necessary to get editable packages working
properly, but it all works.