Jason R. Coombs <jar...@jaraco.com> added the comment:

> there's the tuple subclass which pretends to be a dict.

There's no tuple subclass that's pretending to be a dict. It overrides 
__getitem__ for convenience. It never claims to support Mapping.

> mypy complains about incorrect types in overrides for both.

I'm unsure what the concern is. If there's an issue, it hasn't been reported to 
the project. importlib_metadata runs mypy tests with the test suite (all 
passing) and [twine uses the API with strict mypy checking 
enabled](https://github.com/pypa/twine/blob/eff3a454df49c6e998d3d21d07ef846d8318e446/twine/cli.py#L43-L69)
 without any exclusions.

> the worst part of this is that the `__getitem__` moves from O(1) to O(N) (in 
> some private code this makes importlib.metadata on 3.10 _10x slower than on 
> 3.9_)

This issue was revealed during the review and I acknowledged the concern and 
agreed to address the issue if it mattered. This project has demonstrated its 
concern for performance issues as are apparent through a number of 
optimizations in the changelog. In every use case I've seen, the performance is 
improved by the current approach (a group/sort operation is avoided). If the 
performance is a concern, I once again welcome a bug report describing the 
use-case and the impact, though I suspect it's an isolated case and likely 
would best be addressed outside the official codebase.

> this is an api break with 3.9 which returned a `list`

I acknowledge this break, though I believe the concerns are overblown. The API 
specifically sought to reduce dependence on receiving a list and instead to 
provide a more abstract collection. 

> I don't think [introducing behavior in backports] is appropriate.

It's true, the "backport" monkier is a false one here. From the very beginning, 
these modules first introduced their behavior outside the stdlib and were then 
ported into CPython. Moreover, the past couple of years have seen substantial 
refinement and innovation and was able to move much faster and reach stability 
much faster and with wider adoption than if the library had followed the 
stardard Python development cadence.

It's quite likely that this project will eventually stabilize to the point that 
most users do not need the backport, but while it exists, it's providing 
massive value. Consider the most recent example 
(https://importlib-metadata.readthedocs.io/en/latest/history.html#v4-3-1) where 
a performance improvement caused a regression. The regression was detected and 
fixed within a day. Now when CPython adopts that behavior, we can all have 
higher confidence in the viability of the implementation.

It would be a pretty big shift to block this approach, but it's not out of the 
realm of consideration. Still, it's out of scope for this discussion. Feel free 
to raise it separately.

> [the typing issues] were *trivially solved* by a dictionary comprehension

No such solution was proposed by anybody, but more importantly, I don't believe 
the solution would have been so trivial and still met the objectives.

> the types describing the new apis require *significant* `# type: ignore`s to 
> pass mypy because they violate basic substitutability.

I'm unaware of this issue and it's not been reported, but I also don't believe 
it's an issue. Both twine and keyring have adopted the latest API and pass mypy 
tests.

> they also cannot be used in any of the contexts they were appropriate for in 
> <3.10 (Dict[str, List[EntryPoint]] or List[Entrypoint] depending on the api).

That's right. The API has changed.

> many issues on importlib-metadata

Where "many" ~= 1 
(https://github.com/python/importlib_metadata/issues?q=is%3Aissue+DeprecationWarning).

> many issues linking to your deprecation issue

Do you mean https://github.com/python/importlib_metadata/pull/289 or something 
else? I see ~4 projects (astropy, pytest-randomly, keyring, virtualenv) making 
mention there. I'd expected the number of projects to be affected to be more 
than that.

> there is significant toil

I care about toil. A lot. I don't make incompatible changes lightly, and I 
spent a good deal of time documenting the motivations and providing guidance on 
how to transition. I've actively worked with each project that's requested help 
to minimize their toil and provide a one-shot transition to the new API.

> if you look at your issue tracker it has been reported before and by others

I looked and didn't find it. Help me see what I'm missing.

> "the testsuite didn't demonstrate this usecase so I'm free to change it"

That's not the spirit of my words. The API had an intended usage that was borne 
out by the documentation and tests. If users relied on other interfaces that 
were incidentally present, the user bears some risk in relying on those 
behaviors. Still, I accept responsibility to provide a transitional support 
even for those cases.

----------
nosy:  -gaborjbernat

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue44246>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to