GeezerVison

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

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.

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

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.

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.

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:

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.