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

pep508: add MarkerTree::negate #4996

Merged
merged 2 commits into from
Jul 12, 2024
Merged

pep508: add MarkerTree::negate #4996

merged 2 commits into from
Jul 12, 2024

Conversation

BurntSushi
Copy link
Member

@BurntSushi BurntSushi commented Jul 11, 2024

This does pretty much what you might expect: it takes an existing
MarkerTree and returns a new one that evaluates to the opposite of the
marker given for all possible marker environments. For the most part.

There are two main hitches here.

The first is that some MarkerTree values are nonsense. For example,
"foo" == "bar" and sys_platform ~= "linux". Both such expressions
always return false today. Technically, a logical negation would imply
that they would always return true in the result. But this would imply
that negation turns nonsense into sense. We shouldn't do that. So the
negation of nonsense remains nonsense.

The second is that negating compatible version constraints like
python_version ~= "3.6" cannot be done in a single marker expression.
Instead, the negation is itself a disjunction of negations. In this
case, python_version < 3.6 or python_version != 3.6.*.

I've added some basic tests to cover this new operation.

The main motivation for adding a negation operation is in service of
fixing bugs #4732, #4687, #4640 and #4992.

It's unclear to me whether this was intentional or not, but
I realized that converting a MarkerExpression to a string
treated EqualStar and NotEqualStar as Equal and NotEqual,
respectively. I tweaked this to match the Display impl for
VersionSpecifier.

(Negation tests in the next commit cover this change.)
It does what you think it does, for the most part.
Copy link
Member

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

Nice job :)

Copy link
Member

@ibraheemdev ibraheemdev left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Nice work, that's a very elegant solution!

@BurntSushi BurntSushi merged commit 8d6c49b into main Jul 12, 2024
49 checks passed
@BurntSushi BurntSushi deleted the ag/marker-negation branch July 12, 2024 11:37
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.

4 participants