Enforce local versions of objects

[jaraco]

If a provider is returning objects from importlib.metadata (Distribution, Message, PackagePath), replace them with local versions for predictable types for consumers and type checkers.

Closes #486; supersedes #487

• Add a couple of functions from jaraco.functools.
• Create 'localize_dist' function to convert stdlib to local versions of Distributions.
• Create 'localize_metadata' function to normalize the outputs for .metadata.
• Create 'localize_package_path' function to convert stdlib to local versions of PackagePath.

[jaraco]

@abravalheri What do you think about this approach compared to #487?

The nice thing about this approach is it provides simpler type expectations for downstream consumers while also not imposing any restrictions on providers.

There's still a failing mypy check that I need to resolve, but the tests pass otherwise.

Also, it's a little bit ugly that objects that can't be converted are cast instead, which is technically incorrect, but should provide a low-disruption signal to address those objects. I'll flag that in a code comment.

[jaraco]

This block is perhaps the least respectable part of the proposed change. It means that any Distribution subclasses that aren't derived from importlib_metadata.Distribution or importlib.metadata.PathDistribution (e.g. a custom subclass of importlib.metadata.Distribution) will get a warning and then will possibly not comply with the interface. It's no worse than the status quo, however, except that there's no indication that a type may not be complying with an interface. At least now, a warning will be issued.

The proper fix will be for providers to supply importlib_metadata.Distribution subclasses when importlib_metadata is present.

[abravalheri]

Thank you very much for having a look on this. The solution looks good for me, although I share the feeling that the simple cast may be a little fragile.

The proper fix will be for providers to supply importlib_metadata.Distribution subclasses when importlib_metadata is present.

Should this be officially communicated somehow to the developers? I believe that there is some opposition against using a try..except if importlib_metadata is not included explicitly as a package dependency. We can see an example of that feeling in the thread starting with pypa/pyproject-hooks#195 (comment).

[abravalheri]

Not sure how the core devs would feel about adding this as a "tip" or recommendation in the importlib.metadata docs...

[jaraco]

Should this be officially communicated somehow to the developers?

Yes, probably.

Not sure how the core devs would feel about adding this as a "tip" or recommendation in the importlib.metadata docs...

It should go wherever the docs are that provide guidance for providers. I'll figure out where that is and include it here or link it.

[jaraco]

In python/cpython#123976, I've filed an issue upstream to track the documentation, but as I think more about what the documentation says, I'm not confident in the approach. I'll continue the conversation in #486.

[jaraco]

Tests are passing now on all but Python 3.8 and 3.9. They're failing on the project unit (pytest-checkdocs). It seems that the warning functionality is somehow getting triggered for pytest-checkdocs even though the class is of the intended type:

.::project
tests/data/sources/example2::project
  /home/runner/work/importlib_metadata/importlib_metadata/importlib_metadata/_compat.py:80: UserWarning: Unrecognized distribution subclass <class 'importlib_metadata.PathDistribution'>
    warnings.warn(f"Unrecognized distribution subclass {dist.__class__}")

Surely importlib_metadata.PathDistribution is an instance of importlib_metadata.Distribution. I need to figure out what's causing that not to be the case.

[jaraco]

I think there are two issues going on:

First, when running under pytest, some modules (namely plugins) get imported early, before assertion rewriting is turned on. That includes pytest_checkdocs, which imports jaraco.packaging which imports build, which imports importlib_metadata. When later importlib_metadata is re-imported with the assert-rewrite support, a new copy is imported such that the types aren't comparable using isinstance:

> /Users/jaraco/code/python/importlib_metadata/importlib_metadata/_compat.py(82)localize_dist()
-> return cast(importlib_metadata.Distribution, dist)
(Pdb) importlib_metadata
<module 'importlib_metadata' from '/Users/jaraco/code/python/importlib_metadata/importlib_metadata/__init__.py'>
(Pdb) build._compat.importlib.metadata
<module 'importlib_metadata' from '/Users/jaraco/code/python/importlib_metadata/importlib_metadata/__init__.py'>
(Pdb) importlib_metadata is build._compat.importlib.metadata
False

A second, related issue, is that the same issue happens during localize_metadata. Because the isinstance check fails, the importlib_metadata._adapters.Message object gets re-constructed from an existing message (of the same class), causing the redent function to be invoked twice on the Description, mangling the reStructuredText (by adding ' '*8 at the beginning).

[jaraco]

I notice there is some small impact on performance:

exercises.py:cached distribution: 0:00:00.000302 (+0:00:00.000007, 2%)
exercises.py:discovery: 0:00:00.000300 (+0:00:00.000003, 1%)
exercises.py:entry_points(): 0:00:00.002260 (+0:00:00.000040, 2%)
exercises.py:entrypoint_regexp_perf: 0:00:00.000059 (+0:00:00.000001, 2%)
exercises.py:uncached distribution: 0:00:00.000452 (+0:00:00.000002, 0%)

Nothing too scary.

[jaraco]

I think there are two issues going on:

I confirmed that running pytest --assert plain against 1a30e01 (prior to the workarounds), the tests pass, confirming that the issue was indeed isolated to the pytest assert rewrite importer.