-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add/improve support for Path dependencies and GIT dependencies #712
Conversation
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Need to add a test for subdirectory. |
I suspect the micromamba test failures may be due to the release of micromamba 2.0 |
Noooooo! 😱 I wanted to release! Ok, I guess we have more work. Regarding this PR, it looks mostly great. One of my big concerns is favoring Poetry-style parse_python_requirement("/path/to/some-package", manager="pip", mapping_url=DEFAULT_MAPPING_URL) Looking forward to the additional test for |
Ok, micromamba v2 was not so bad. I believe I took care of it in #713. Feel free to rebase on main, and I hope that will fix things. |
247d1cb
to
8fa77b4
Compare
Hopefully that works -- the PEP for local requirements seems to be |
On first glance this looks great, thanks so much @romain-intel!!! Note one hiccup in the doctest with a missing |
whoops. Sorry about that. Hopefully that will do it. Unrelated: I see a flurry of activity, planning a release soon? |
Gah -- something up with windows :( . I can't seem to retrigger tests but I think everything else should be fine :). |
Oops, #717 should fix it. Could you please rebase again? |
Yes, I really want to get this released as soon as possible. There are a few tricky things though. The release of micromamba 2 is still causing a lot of chaos, e.g. #709, so we should probably let that settle down a bit first. |
Specifically: - adds subdirectory for GIT dependencies - adds the ability to pass "path" dependencies to Poetry
70c4159
to
c65d095
Compare
Rebased. And yes, micromamba v2 is also causing me headaches :). Hopefully you can release soon though. Definitely interested in getting this large change integrated. |
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.
This looks good to me, are you ready to merge @romain-intel?
I'm going to take all the green I can get and merge this. Feel free to do any followup in a subsequent PR. |
Thanks -- yes, sorry, busy day. Thanks for merging!! |
I had a concern regarding the We're assuming a nice PEP 508 compliant input string where the name is specified. However, pip is unfortunately a bit more loose about actually specifying the name. So if I do: from conda_lock.lookup import DEFAULT_MAPPING_URL
from conda_lock.src_parser.pyproject_toml import parse_python_requirement
parse_python_requirement(
"./downloads/numpy-1.9.2-cp34-none-win32.whl", mapping_url=DEFAULT_MAPPING_URL
) then I get
That error message is a lot more informative than I was expecting, which is why I don't think this is so serious of a problem. Since not everyone has recently read PEP 508, it might be kind to intercept this exception and reraise with a note explaining that names can be specified by |
I can make a more informative message. I'll try this over the weekend or early next week. And yes, I am aware of that and it is spec compliant if not the easiest. I think pip extracts the name from the setup or something when not specified. So nice but then again, why have specs :). |
Specifically:
Description
This simply extends support of the arguments that conda-lock can parse in a pyproject.toml and passes them down to Poetry. Specifically it adds support for local directories and wheels as well as support for #subdirectory in GIT packages.