Skip to content

Conversation

@Liam-DeVoe
Copy link
Contributor

Closes #767.

@notatallshaw
Copy link
Member

notatallshaw commented Sep 13, 2025

I'm supportive of this PR, I believe it makes packaging closer to PEP 440 compliant, as invalid versions are expressible via arbitrary equality.

You've added tests for Specifier.contains, SpecifierSet.contains, and SpecifierSet.filter, but you are missing tests for Specifier.filter which should be the same ones you added to TestSpecifierSet.test_specifier_filter but for TestSpecifier.test_specifier_filter.

@Liam-DeVoe
Copy link
Contributor Author

Liam-DeVoe commented Oct 4, 2025

Any movement on this and/or #931? I think these are relatively straightforward changes; and if you'd like to see a different approach instead then I'm happy to do so. I'd love to contribute more to packaging and so I've tried to ease myself in with these contributions, but it's hard to motivate myself to contribute more if PRs sit for this long!

@notatallshaw
Copy link
Member

It occurs to me that his will likely need a follow up PR where these method handle arbitrary equality, I've wrote up an issue #943

@notatallshaw notatallshaw self-requested a review October 23, 2025 23:16
@notatallshaw
Copy link
Member

Hi @Liam-DeVoe I just recently became a packaging maintainer and I am willing to review and accept this. Unfortunately though I just merged another PR that has caused this PR to have conflicts, if you are okay fixing those conflicts I will review afterward.

@Liam-DeVoe
Copy link
Contributor Author

Congrats! Yes of course, happy to!

@Liam-DeVoe
Copy link
Contributor Author

(that CI error is a real mypy warning that should have been an error if I had written better tests - I'll investigate and add a covering test)

@Liam-DeVoe
Copy link
Contributor Author

I'm actually confused as to why #767 says that ===foobar should return False for foobar. On my reading of PEP 440, it should match:

An example would be ===foobar which would match a version of foobar.

I think this is what you were pointing out as well in #943 (comment). I can make this PR return False, so that at least it's not erroring, and then I'm guessing you're already planning a followup to properly handle arbitrary equality and return True instead.

@notatallshaw
Copy link
Member

@Liam-DeVoe I agree, it's up to you, you can either fix it or I will make a follow up PR, once I am done with the pip 25.3 release, that fixes it.

@Liam-DeVoe
Copy link
Contributor Author

I don't think I'll have immediate time to dig into this in the next few days, so I would say you should go for it.

@notatallshaw notatallshaw merged commit 9983e4b into pypa:main Oct 25, 2025
39 checks passed
@notatallshaw
Copy link
Member

Thanks @Liam-DeVoe for your contribution to packaging.

@Liam-DeVoe
Copy link
Contributor Author

Thank you Damian! I'm hopeful I can continue to contribute where it makes sense.

@Liam-DeVoe Liam-DeVoe deleted the contains-invalid-version branch October 26, 2025 02:19
@notatallshaw
Copy link
Member

notatallshaw commented Nov 1, 2025

@Liam-DeVoe I've found an interesting edge case adding support for arbitrary equality of arbitrary strings:

SpecifierSet("===foobar,!=1.0").contains("foobar")

Should that be True? And if that's true shouldn't also the following be True:

Specifier("!=1.0").contains("foobar")

I'm tempted to say yes for both and special case != here and not filter invalid versions when there are only != operators.

@notatallshaw
Copy link
Member

I've made #954 that change the behavior you implemented here, just for empty specifiers and not equal to specifiers.

@Liam-DeVoe
Copy link
Contributor Author

I'm almost tempted to propose that arbitrary equality clauses should not be allowed to appear in the same specifier set as any other clause (ie it must appear alone). === matches exactly one string and one string only, so given a specifier set s with an arbitrary equality clause that matches v, adding another clause c can only either (1) cause s to continue to match v, or (2) cause s to match nothing.

But if ruling out the combination of arbitrary equality clauses with other clauses is not an option, then:


I don't think it's sane to have assert Specifier("!=1.0").contains("foobar"). This might lead to accidentally accepting invalid versions. The way I think of specifier set and arbitrary equality is that SpecifierSet starts with the universe of valid versions. Each non-arbitrary-equality clause added to the specifier set restricts the universe of versions. Adding an arbitrary equality clause has unique semantics, in that it widens the universe of versions to include exactly that version, regardless of what any other clause says.

I know this breaks this very nice invariant:

For any version v and specifiers A and B, if v is not satisfied by A, then v must also not be satisfied by A and B.

But I don't think it's too bad to repair. I would state the following two invariants:

For any valid version v and specifiers A and B, if v is not satisfied by A, then v must also not be satisfied by A and B.

For any invalid version v and specifiers A and B, if v is satisfied by A, then v must also be satisfied by A and B.

Though this second invariant needs some work, e.g. what happens when a specifier set includes two different arbitrary equality clauses. I think disallowing multiple arbitrary equality clauses in the same specifier set would be a fine solution.

@notatallshaw
Copy link
Member

notatallshaw commented Nov 2, 2025

I'm almost tempted to propose that arbitrary equality clauses should not be allowed to appear in the same specifier set as any other clause (ie it must appear alone). === matches exactly one string and one string only, so given a specifier set s with an arbitrary equality clause that matches v, adding another clause c can only either (1) cause s to continue to match v, or (2) cause s to match nothing.

PEP 440 says nothing about specifier sets, so the semantics of sets is the prerogative of packaging, however I don't want to break backwards compatibility, and this is allowed today, e.g.

>>> SpecifierSet('===1.0,>0.5').contains('1.0')
True

I don't think it's sane to have assert Specifier("!=1.0").contains("foobar"). This might lead to accidentally accepting invalid versions. The way I think of specifier set and arbitrary equality is that SpecifierSet starts with the universe of valid versions. Each non-arbitrary-equality clause added to the specifier set restricts the universe of versions. Adding an arbitrary equality clause has unique semantics, in that it widens the universe of versions to include exactly that version, regardless of what any other clause says.

I am okay with the choice that the specifier !=1.0 only acts on valid versions, not arbitrary strings, and therefore it excludes all arbitrary strings that don't conform to valid version semantics. I do think it reads awkwardly, but it is justifiable.

But I am not okay to say that SpecifierSet("===foo") has special semantics that expands the universe of versions beyond the empty specifier set SpecifierSet("") as I will explain.

If the goal is to not allow users to include arbitrary strings I think we should go back to them throwing an exception and then users can prevalidate their versions with Version. One of my goals here is to conform to the Python packaging spec which does allow for arbitrary strings as the result of valid specifiers.

But I don't think it's too bad to repair. I would state the following two invariants:

For any valid version v and specifiers A and B, if v is not satisfied by A, then v must also not be satisfied by A and B.

For any invalid version v and specifiers A and B, if v is satisfied by A, then v must also be satisfied by A and B.

I don't think this second invariant holds, but even if it did it still wouldn't achieve my other main goal.

I want to be able to consider specifier sets without considering what versions they act on, because I want to be able to implement resolution algorithms that can do some static analysis on the specifiers, and looking up what versions available is potentially an IO call.

I've already spent a lot of time reading the spec and conforming packaging to make sure pre-release specifiers don't have this "universe expanding" property, I am going to do my best to avoid it for arbitrary string versions also.

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.

Permit contains calls to Specifier[Set] to use arbitrary strings

2 participants