type annotations

[dimbleby]

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:

• joinpath on the SimplePath seems to have been just wrong
• I've annotated some Iterables as Iterators, because (i) they are and (ii) Distribution.from_name() relies on this when it calls next(..) rather than next(iter(..))

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]

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

[dimbleby]

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]

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

[jaraco]

This looks good and nearly ready to merge. Just a few concerns to address.

[dimbleby]

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

• prefer Iterable to Iterator (per the docstrings, and leaving other or future implementations a little more freedom)

  • as a result, added an iter() call in a place that was assuming it had an iterator
  • actually I notice now that I've annotated the requires() methods as returning List where the docstring used to say "iterator" (definitely wrong) and after my search-replace says "iterable" (true, but less specific than list). Any preference on doing anything about that?

• completely ducked the typing of Sectioned.valid(), I don't like either of our suggestions but don't want to get stuck on that

• updated StrPath as suggested

• have Distribution.at() return Distributions rather than committing to PathDistributions

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

[jaraco]

• 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]

Yes. Distribution is the right choice here. Thanks.

the code that you have merged still used PathDistribution:

importlib_metadata/importlib_metadata/__init__.py

Lines 796 to 798 in 705a757

	def find_distributions(
	self, context=DistributionFinder.Context()
	) -> Iterable["PathDistribution"]:

[jaraco]

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]

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