Skip to content

Allow empty specifier in constraints - #8209

Closed
uranusjr wants to merge 1 commit into
pypa:masterfrom
uranusjr:constraint-specifier-empty
Closed

uranusjr wants to merge 1 commit into
pypa:masterfrom
uranusjr:constraint-specifier-empty

Conversation

@uranusjr

@uranusjr uranusjr commented May 8, 2020 •

Copy link
Copy Markdown
Member

A test test_constrained_to_url_install_same_url is currently failing because it supplies a constraint with an empty specifier, which evaluates to false, failing the assertion. (A constraint with an empty specifier won’t actually do anything, so why is this allowed…? But apparently it is allowed.)

InstallRequirement’s type hints say req.specifier is always SpecifierSet, i.e. the assertion is not needed, so I simply dropped it.

@uranusjr
uranusjr requested a review from pfmoore May 8, 2020 21:36
@uranusjr uranusjr added the skip news Does not need a NEWS file entry (eg: trivial changes) label May 8, 2020
@uranusjr

uranusjr commented May 8, 2020

Copy link
Copy Markdown
Member Author

Oh wait, I’m checking test_install_distribution_union_with_constraints and it seems that even the name of a constraint is not guarenteed to exist. Something like this is actually valid:

https://github.com/psf/requests/archive/master.zip[socks]

@uranusjr

uranusjr commented May 8, 2020

Copy link
Copy Markdown
Member Author

Err, the nameless constraint case is much more difficult to deal with. I’ve opened #8210 to discuss this further, but this one can probably go in first.

@pfmoore

pfmoore commented May 15, 2020

Copy link
Copy Markdown
Member

This got covered in my constraint fixup PR, so I don't think it's needed any more.

@uranusjr uranusjr closed this May 15, 2020
@uranusjr
uranusjr deleted the constraint-specifier-empty branch May 15, 2020 16:47
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

skip news Does not need a NEWS file entry (eg: trivial changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants