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

Fix get_python_constraint_from_marker #347

Merged

Conversation

radoering
Copy link
Member

@radoering radoering commented May 8, 2022

  • Added tests for changed code.
  • Updated documentation for changed code.

Fixes get_python_constraint_from_marker() for markers which are not in disjunctive normal form (DNF).

E.g. without the fix:

(python_version < "2.7" or python_full_version >= "3.0.0") and python_full_version < "3.6.0" results in python_constraint *
whereas <2.7 || >=3.0,<3.6 would be correct.

By the way, increased the test coverage for convert_markers() and get_python_constraint_from_marker().

Considering the commits separately may facilitate the review.

Update:

There was another bug in get_python_constraint_from_marker() that is fixed by the last commit.
E.g. for python_version >= "3.9" or sys_platform == "linux" the function returned >=3.9 whereas * would be correct (because for linux all python versions are allowed).

Further, relying on the marker being in DNF before conversion, allows to simplify convert_markers() significantly.

@radoering radoering requested a review from a team May 8, 2022 15:59
@radoering radoering force-pushed the fix-get-python-constraint-from-marker branch from fa94733 to 17169c6 Compare May 8, 2022 16:07
@abn abn mentioned this pull request May 8, 2022
@abn
Copy link
Member

abn commented May 9, 2022

@radoering python-poetry/poetry#5579

src/poetry/core/packages/utils/utils.py Outdated Show resolved Hide resolved
src/poetry/core/packages/dependency.py Outdated Show resolved Hide resolved
radoering added 2 commits May 9, 2022 20:14
…or markers whose disjunctive normal form (DNF) contains a group without marker name "python_version"

By the way, convert_markers() was simplified significantly. That was possible because markers are converted to DNF before conversion.
@radoering radoering force-pushed the fix-get-python-constraint-from-marker branch from a4a4abb to 1183112 Compare May 9, 2022 18:16
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 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 1 Code Smell

No Coverage information No Coverage information
1.6% 1.6% Duplication

@abn abn merged commit dfe7bab into python-poetry:master May 9, 2022
@radoering radoering deleted the fix-get-python-constraint-from-marker branch November 24, 2024 12:39
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.

2 participants