[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.

[jaraco]

A suggestion that came out of documenting this downstream (python/cpython#160344, for python/cpython#123976): consider warning on conformance rather than on class identity.

Today localize.dist warns for any Distribution subclass whose module isn't importlib_metadata:

if dist.__class__.__module__ != 'importlib_metadata':
    warnings.warn(f"Unrecognized distribution subclass {dist.__class__}")

Now that this PR adds IDistribution, that guard could ask whether the object actually satisfies the interface instead. The difference matters for the downstream guidance. As it stands, a provider that implements IDistribution completely still warns, and the person who sees the warning is not the person who can act on it. The custom-provider example in the CPython docs is exactly this case: a DatabaseDistribution that subclasses importlib.metadata.Distribution, implements read_text and locate_file, and inherits the rest. It works correctly under the backport and it warns. So the stdlib text has to explain a warning that providers cannot clear except by depending on this package, which is the coupling #486 set out to avoid. With a conformance check, the guidance collapses to "implement the whole interface", and the warning means something is genuinely missing.

One implementation note, since it is not obvious: isinstance(dist, IDistribution) is not the way to spell it. With requires-python = ">=3.11" and IDistribution carrying property members, isinstance against a runtime-checkable protocol evaluates those properties on 3.11, so the check would read METADATA for every distribution just to decide whether to warn, and a property that raises propagates out of isinstance. Measured on a protocol with metadata and version properties:

Python	properties evaluated by isinstance	property that raises
3.11	metadata, version	exception propagates
3.13	none	no effect

Checking the type avoids that on every supported version, and can name what is missing:

_REQUIRED = (
    'read_text', 'locate_file', 'metadata', 'name',
    'version', 'entry_points', 'files', 'requires',
)

if missing := [name for name in _REQUIRED if not hasattr(type(dist), name)]:
    warnings.warn(
        f"Distribution subclass {dist.__class__} is missing {', '.join(missing)}"
    )

(issubclass is not available here: protocols with non-method members reject it.) This would also subsume the __module__ check that currently works around pytest replacing importlib_metadata, since a replaced class still conforms.

Entirely possible the identity check is deliberate and I am missing a case it protects. The stdlib text works either way; it is just longer, and asks providers to live with a warning rather than to fix something.