Skip to content

Conversation

notatallshaw
Copy link
Member

This is a small follow up to #13581, the name attribute of the Wheel model class is now guaranteed to be canonicalized, this means there are many places where it is now redundant to call canonicalize_name.

To validate this is always true I have set some parameters to use the NormalizedName type from packaging, which is the type of the canonicalized name. I tried to keep this fairly minimal, there were many more places where a canonicalized name is always being used but changing the type on all of them started to make this PR a lot bigger than it needed to be.

We could also use the Version type for the version attribute instead of a str in the Wheel model, which would save a couple of calls to Version elsewhere, but this ended up being non-trivial in the LinkEvaluator.evaluate_link method, so I left it a more brave PR than this one.

@notatallshaw notatallshaw added the skip news Does not need a NEWS file entry (eg: trivial changes) label Sep 20, 2025
Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! FWIW, this makes some small progress towards #7974.

@notatallshaw
Copy link
Member Author

notatallshaw commented Sep 21, 2025

Thanks for doing this! FWIW, this makes some small progress towards #7974.

Interesting, thanks for the info, I'll consider making a follow up PR that more thoroughly minimizes the use of canonicalize_name and maximize using NormalizedName type. But this PR was just meant to be a small follow up from #13581.

@notatallshaw notatallshaw merged commit 3d88026 into pypa:main Sep 21, 2025
28 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2025
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.

3 participants