python / typeshed

Collection of library stubs for Python, with static types
Other
4.39k stars 1.77k forks source link

Fix unnecessary 'pyright: ignore' warning in CI #13101

Closed srittau closed 4 days ago

github-actions[bot] commented 4 days ago

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] commented 4 days ago

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

AlexWaygood commented 4 days ago

Hmm, what problem is this fixing? I see lots of stdlib stubtest failures on recent pushes main, but I don't see pyright failing at all

srittau commented 4 days ago

See here, for example: https://github.com/python/typeshed/actions/runs/12013896148/job/33488465170?pr=11709

Cf. #11709

AlexWaygood commented 4 days ago

Huh, that's very odd... Has it been happening on any other PRs that you know of?

srittau commented 4 days ago

I've merged the latest changes for the unnecessary allowlist entries into a few PRs. Let's see what happens.

AlexWaygood commented 4 days ago

Okay, it's not really a flake: the pyright: ignore is consistently needed on main but consistently not needed on that PR.

It's because that PR adds stubs that depend on django-stubs, and django-stubs depends on tomli. So that PR pulls tomli into the environment and means that pyright can resolve the import in the setuptools stubs. So the pyright failure in that PR is a side effect of the fact that we have no isolation between stubs packages when we test them with pyright in CI -- all non-types dependencies get installed into one big "venv soup".

srittau commented 4 days ago

Indeed. The question is how to solve it. Maybe we just depend on tomli in requirements for Python < 3.11?

AlexWaygood commented 4 days ago

Alternatively... we could maybe just delete this module from the stubs? It seems like an implementation detail at runtime to me; I'm not sure it's meant to be for public consumption. It's probably not worth the bother to maintain stuff like this? @Avasam, what do you think?

github-actions[bot] commented 4 days ago

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Avasam commented 4 days ago

I concur with Alex' assessment of the CI failure.

And yes setuptools.compat is intended to be private https://github.com/pypa/setuptools/pull/4609#issuecomment-2316801191 & https://github.com/jaraco/skeleton/blob/gh-pages/index.md#compatibility-modules Unless we have a niche case where a type in setuptools.compat is used for typing and the two branches differs in a meaningful way, I think we can remove these modules from typeshed.