Skip to content
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 constraint_regions #381

Merged
merged 4 commits into from
May 29, 2022
Merged

Conversation

dimbleby
Copy link
Contributor

@dimbleby dimbleby commented May 28, 2022

This adds constraint_regions() which is a utility that takes a set of python version constraints and spits out the distinct regions that those constraints split the world into.

For instance: input >=3.6 and >=2.7,<3.0.0 || >=3.4.0
output <2.7, >=2.7,<3.0.0, >=3.0.0,<3.4.0, >=3.4.0,<3.6, >=3.6.

I am expecting views on whether this is the right place for such code to live, and what the method should be called, and requests for unit tests, which we can get to in due course...

The motivation for this is that I think we need it to fix a particularly icky poetry export example python-poetry/poetry-plugin-export#32. There, the tree-walk at some point finds the requirement:

{version = "*", markers = "python_version > \"3.6\" and implementation_name != \"pypy\""}

and correctly decides that only ipython 7.16.3 can satisfy this. But that causes it to add a ... or python_version >= "3.6" ... marker for ipython 7.16.3, which sadly overlaps with the markers on a newer ipython version.

My intended fix is to split up the python version space and when adding a requirement during the walk, actually add several requirements, one for each range of python versions. That allows the locker to limit the ipython 7.16.3 selection to >3.6,<=3.7 and all is right again.

(I'll follow up with an MR in poetry, hopefully clarifying the above).

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First thought: Is that any different from building the union and returning its ranges?

After trying the last test case: Ahh, that's what it's for. (Should have read your description more thoroughly. 😉)

The implementation seems correct to me and the unit tests are fine. I don't have a better name or place for that function either. I just want to wait a bit if there are other opinions.

src/poetry/core/semver/util.py Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented May 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@radoering radoering merged commit 63322f6 into python-poetry:main May 29, 2022
@dimbleby dimbleby deleted the constraint-regions branch May 29, 2022 20:22
bostonrwalker pushed a commit to bostonrwalker/poetry-core that referenced this pull request Aug 29, 2022
* Add constraint_regions

* unit test constraint_regions

* _ranges_for() and flatten are the same
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants