python / importlib_metadata

Library to access metadata for Python packages
https://importlib-metadata.readthedocs.io
Apache License 2.0
122 stars 79 forks source link

type annotations #449

Closed dimbleby closed 1 year ago

dimbleby commented 1 year ago

It's kind of annoying that this library publishes a py.typed, but most of the API is not type-annotated. Users who check their own code with mypy are obliged to scatter around # type: ignore[no-untyped-call] comments.

Possible points of interest:

This MR doesn't annotate the whole API: I've ducked the slightly difficult ones like distributions(), select() and matches(). typeshed has annotations that are presumably satisfactory in practice, but it looks as though applying them here would be more invasive than I intend to be in this commit. https://github.com/python/typeshed/blob/main/stdlib/importlib/metadata/__init__.pyi

This MR is a long way from annotating the whole project - mypy --strict importlib_metadata reports 171 errors, so it would take a bit more of a campaign to work through that.

jaraco commented 1 year ago

FYI, @RonnyPfannschmidt was also considering tackling this effort (discussion in #importlib-metadata channel on PyPA discord).

dimbleby commented 1 year ago

Cool. I'm mostly just aiming for the low-hanging fruit here and perhaps testing the waters for receptiveness to further work.

The pipeline and I both are currently happy; I reckon I'm done fiddling with this one now.

RonnyPfannschmidt commented 1 year ago

I have not yet started own work, great to see this start

dimbleby commented 1 year ago

OK, I've pushed change that - I hope - err in the direction of avoiding and deferring any difficult questions

jaraco commented 1 year ago
  • just spotted MetadataPathFinder.find_distributions() - is it ok for that to commit to PathDistribution or does that want changing too?

Yes. Distribution is the right choice here. Thanks.

dimbleby commented 1 year ago

Yes. Distribution is the right choice here. Thanks.

the code that you have merged still used PathDistribution: https://github.com/python/importlib_metadata/blob/705a7571ec7c5abec4d4b008da3a58df7e5c94e7/importlib_metadata/__init__.py#L796-L798

jaraco commented 1 year ago

Yes. Distribution is the right choice here. Thanks.

the code that you have merged still used PathDistribution:

Aah. I was looking at the wrong thing (because it wasn't in the diff). I think PathDistribution is okay here because it's a specific implementation that does specifically only return PathDistribution (even though other DistributionFinders might return a more generic Distribution). Thanks for flagging it.

jaraco commented 7 months ago

I realized that the declaration of Distribution.locate_file is incorrect and have started work to correct it in #480.