Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metaclass issues in stubs for Redis #9127

Closed
github-actions bot opened this issue Nov 8, 2022 · 8 comments
Closed

Metaclass issues in stubs for Redis #9127

github-actions bot opened this issue Nov 8, 2022 · 8 comments
Labels
help wanted An actionable problem of low to medium complexity where a PR would be very welcome topic: redis Issues related to the redis third-party stubs

Comments

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Runs listed here: https://github.com/python/typeshed/actions/workflows/daily.yml

@github-actions github-actions bot added the help wanted An actionable problem of low to medium complexity where a PR would be very welcome label Nov 8, 2022
@JelleZijlstra
Copy link
Member

Metaclass stuff in various third-party packages, I guess from the new mypy release. #9123's CI should ideally have caught it.

@sobolevn
Copy link
Member

sobolevn commented Nov 8, 2022

I am working on fixing metaclasses stuff.
During the process I also can double check that this feature is:

  1. Correct
  2. Useful

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 8, 2022

Two thoughts:

  • We may as well just skip stubtest for cryptography at this point. Those stubs are obsolete, pinned to a very old version, and will be deleted as soon as other typeshed stubs can depend on upstream cryptography rather than types-cryptography (Remove cryptography (depends on #5952) #5618)
  • There are so many errors for SQLAlchemy and Redis, that maybe we should just allowlist all the metaclass errors for those libraries for now. We can then work through the allowlist at our own pace, without worrying about stubtest failing on a daily basis.

@srittau srittau changed the title Daily tests failed on Tue Nov 08 2022 Metaclass issues with mypy 0.900 Nov 8, 2022
@srittau srittau changed the title Metaclass issues with mypy 0.900 Metaclass issues with mypy 0.900 (daily tests failure) Nov 8, 2022
@sobolevn
Copy link
Member

sobolevn commented Nov 8, 2022

Observations:

  1. This error does not look good:
 error: invoke.parser.ParseMachine is inconsistent, metaclass differs
Stub: at line 15 in file /home/runner/work/typeshed/typeshed/stubs/invoke/invoke/parser/__init__.pyi
N/A
Runtime: at line 176 in file /tmp/tmpswurkl33/lib/python3.9/site-packages/invoke/parser/parser.py
<class 'invoke.vendor.fluidity.machine.MetaStateMachine'>

error: invoke.parser.parser.ParseMachine is inconsistent, metaclass differs
Stub: at line 15 in file /home/runner/work/typeshed/typeshed/stubs/invoke/invoke/parser/parser.pyi
N/A
Runtime: at line 176 in file /tmp/tmpswurkl33/lib/python3.9/site-packages/invoke/parser/parser.py
<class 'invoke.vendor.fluidity.machine.MetaStateMachine'>

It is the same object, but re-exported in different modules. No need to report it twice.

  1. We now have a lot of stuff in allowlists for some modules. I think it might be a good idea to add error codes. Right now all errors might be ignored.

  2. Problem with types that do not have abstract methods and still have ABCMeta metaclass. Might be a good idea to special case this.

@AlexWaygood
Copy link
Member

It is the same object, but re-exported in different modules. No need to report it twice.

Stubtest does that with any object that's re-exported in another module. (It's not ideal behavior, but it's not unique to the new metaclass-related check.)

@AlexWaygood
Copy link
Member

  • There are so many errors for SQLAlchemy and Redis, that maybe we should just allowlist all the metaclass errors for those libraries for now. We can then work through the allowlist at our own pace, without worrying about stubtest failing on a daily basis.

To update on the status here: I believe @Avasam fixed all the SQLAlchemy metaclass issues. There may still be quite a few metaclass-related allowlist entries for our Redis stubs that we could fix, however.

@AlexWaygood AlexWaygood changed the title Metaclass issues with mypy 0.900 (daily tests failure) Metaclass issues in stubs for Redis Apr 29, 2023
@Avasam
Copy link
Collaborator

Avasam commented Apr 30, 2023

Concerning Redis: Subclassing CommandsProtocol like the source does fix the differing metadata issue, however CommandsProtocol has a member connection_pool that is not implemented by its subclasses.
I've open an issue about this upstream: redis/redis-py#2730 and redis/redis-py#2729
Not sure what would be the best way to tackle this in typeshed.

@srittau
Copy link
Collaborator

srittau commented Aug 1, 2024

The redis stubs are scheduled for removal (see #10592) and only support redis 4. While we still accept contributions, I'm closing this issue now for housekeeping purposes. Improvements to the types of redis 5 should be directed to the redis-py repository.

@srittau srittau closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted An actionable problem of low to medium complexity where a PR would be very welcome topic: redis Issues related to the redis third-party stubs
Projects
None yet
Development

No branches or pull requests

5 participants