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

Allow case insensitive marker evaluation #508

Conversation

jeroendecroos
Copy link
Contributor

@jeroendecroos jeroendecroos commented Feb 8, 2022

Closes #507

Not sure if this is the ideal way to solve the problem, but I've run the ´nox -s tests-3.10´ tests and they all pass, including the new tests to demonstrate the bug.

@jeroendecroos jeroendecroos changed the title 507 case insensiteve marker evaluation Allow case insensitive marker evaluation (#507) Feb 8, 2022
@brettcannon
Copy link
Member

I don't know this code well enough to do a review right now, but do note that linting failed.

@jeroendecroos
Copy link
Contributor Author

Linting is now passing, I assume that the other check will pass again, but I'll double check tomorrow.

@jeroendecroos
Copy link
Contributor Author

All checks passed

@pradyunsg pradyunsg changed the title Allow case insensitive marker evaluation (#507) Allow case insensitive marker evaluation Feb 16, 2022
@pradyunsg
Copy link
Member

This would deviate from what PEP 508 states:

Comparisons in marker expressions are typed by the comparison operator. The <marker_op> operators that are not in <version_cmp> perform the same as they do for strings in Python. The <version_cmp> operators use the PEP 440 version comparison rules when those are defined (that is when both sides have a valid version specifier). If there is no defined PEP 440 behaviour and the operator exists in Python, then the operator falls back to the Python behaviour.

The behaviour of == should not be changed in the manner being proposed here.

IIUC, the motivation here is similar to what is being discussed in https://discuss.python.org/t/7614/ -- and the resolution will likely go down the same path as that discussion. x-ref pypa/pip#10889 as well.


Two GitHub workflow notes:

@uranusjr
Copy link
Member

Note that marker casing should no longer be a problem with metadata version 2.3 because PEP 685 mandates the extras must be normalised in both sides of the marker expression.

@brettcannon
Copy link
Member

@jeroendecroos did you want to try and resolve the merge conflicts and address the comment from Pradyun? And is this still a concern with the changes made to support PEP 685?

@jeroendecroos
Copy link
Contributor Author

Hi @brettcannon

I saw that 53dbb25 should provide the functionality required.

I've ran my tests with only those changes and the test has passed. I think the tests added in that commit should be sufficient to verify my use case so we can close this PR and ticket according to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should markers ignore case when comparing packages?
4 participants