Conversation
|
Hey @baszalmstra, I know you have spent some time thinking about conditional marker conversion. Would you mind please taking a quick peak at the |
|
I think the best way to test this is to have a large set of conditions/requirements convert them to conda requirements and snapshot them. That makes it easy to see what the conversion will do in many different cases, whether it makes, and whether it regressed. (You might already have that, I might have missed it) |
|
Hey @baszalmstra, with the changes here: conda-incubator/conda-pypi-test#19, I ran |
Merging this PR will degrade performance by 30.24%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| 👁 | test_build_conda[imagesize] |
51.1 ms | 73.3 ms | -30.24% |
Comparing danyeaw:v3-syntax (d94bc67) with main (836a0f3)1
Footnotes
ryanskeith
left a comment
There was a problem hiding this comment.
Overall this looks good to me. Sadly, the best way to get the parsed atoms from the markers for easy parsing is through a private member function. Another way would be to use another private function from packaging either
from packaging._parser import parse_marker
or
from packaging._parser import parse_requirements
Either would give the same parsed _markers result that is being used. Having no public programmatic access to the individual parts has has been marked as an issue for a few years in the packaging module.
A few comments that showed examples code be helpful for future reviewers to quickly understand the intent of the transform.
Co-authored-by: Ryan Keith <rkeith@anaconda.com>
Description
Closes #247. In py-rattler 0.23.0, they updated support for the v3 repodata with conditionals/extras which is defined in:
Currently the conditional/extras CEP draft defines the extras field to be called
extras. py-rattler is using the fielddepends_extra, so I am going with that to match the implementation and I posted about this on Zulip.This also enabled usage of conditional markers which requires quite a lot of conversion:
python...MatchSpec fragments, includingpython_version not in "x, y"->(python!=x and python!=y).__win,__linux,__osx,__unix).extra_depends, with remaining non-extra marker logic encoded via[when="..."].One thing I'm not sure about is that I am using a private
_markermethod from the packaging library in one place. I tried to keep the usage isolated to one area, and it is for only test metadata generation. I think this is much better than using regex to parse the markers.Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?