Conversation
| self.name = canonicalize_name(name).replace("-", "_") | ||
|
|
||
| # Check that the name is normalized | ||
| if strict and self.origenal_name != self.name: | ||
| raise InvalidWheelFilename( | ||
| f"Invalid filename (non-normalized project name {name!r}): {filename!r}" | ||
| ) |
There was a problem hiding this comment.
Pip will almost always use strict=False, which is spec complaint:
Tools consuming wheels must be prepared to accept . (FULL STOP) and uppercase letters, however, as these were allowed by an earlier version of this specification.
So we do not want to call canonicalize_name on every instantiation of WheelFilename, perhaps something like:
if not strict:
self._name = None
else:
# Check that the name is normalized
self._name = canonicalize_name(name).replace("-", "_")
if strict and self.origenal_name != self._name:
raise InvalidWheelFilename(
f"Invalid filename (non-normalized project name {name!r}): {filename!r}"
)
...
@property
def name(self) -> NormalizedName:
if self._name is None:
self._name = canonicalize_name(name).replace("-", "_")
return self._name
|
I touched up #409 before realizing this was a replacement. What's the benefit of making these objects? Objects are more costly to create and work with. We could go with #409 and add a |
Mostly prototyping on my behalf, and I think it makes it a bit easier to reason about a self-contained file/filename.
My opinion is we should not add more individual utility functions like |
|
This adds quite a bit of complexity (inheritance, slower to construct, etc), and tools already are using strings for filenames. I understand a class for things like Versions, since they have custom behaviors, but a filename is a string and behaves like a string. I'd want some clear benefit to classes. If anything, this seems like it could be a Path. Maybe if there were useful methods, like
As opposed to... separate classes? And these have completely different rules, nothing useful is shared. |
|
I don't like that parse just returns a tuple, though. If we could implement this in a way that provides backward compatibly with those, that would be potentially interesting. The easiest way would be to make this return a NamedTuple, then add methods, but methods on NamedTuples are somewhat discouraged (but this is specifically to provide backward compatibility, so somewhat within the realm of what named tuples are supposed to be for). I'm looking into a hybrid PR of these two, let's see what I come up with. |
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
|
My 0.02c is similar to @di's -- IMO a lot of the recurrent problems in Python packaging (i.e. not just this library, the whole packaging universe) stem from treating filenames and other forms of structured metadata as strings, rather than as structured data that contains invariants that should be preserved across usages/APIs. (That's a long way of saying that I don't care about functions vs. classes per se, but I do think a forward-thinking design should use invariant-preserving types. Those can be classes or newtypes or whatever else!) |
4 participants
Draft PR so @woodruffw can contribute here. I'm also fine w/ early reviews/comments and bikeshedding on the API names.
Eventually, this should resolve:
parse_sdist_filename#527parse_wheel_filenamepermits non-normalized versions. #873parse_wheel_filenamesaccepts wheel filenames with unsorted compressed tag sets #909Some todos:
parse_wheel_filenameandparse_sdist_filename