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

Remove redis (not before February 2025) #10592

Closed
srittau opened this issue Aug 16, 2023 · 20 comments · Fixed by #13157
Closed

Remove redis (not before February 2025) #10592

srittau opened this issue Aug 16, 2023 · 20 comments · Fixed by #13157
Labels
stubs: removal Pending removal of third-party distributions topic: redis Issues related to the redis third-party stubs

Comments

@srittau
Copy link
Collaborator

srittau commented Aug 16, 2023


Deferred: As of redis-py 5.0.3 the upstream annotations are lacking. We should not remove these stubs, until the upstream annotations reach parity with the stubs.

@srittau srittau added the stubs: removal Pending removal of third-party distributions label Aug 16, 2023
@srittau
Copy link
Collaborator Author

srittau commented Aug 21, 2023

Redis 5.0.0 added a py.typed file. Still, I wonder whether we should remove our stubs at this point, as the redis code is basically untyped, even quite fundamental classes and methods.

@AlexWaygood
Copy link
Member

Yes, I'm pretty unsure what the best course of action is for our users here.

On the one hand, we generally remove stubs as soon as a library adds a py.typed file. I think that's the correct general policy: if a library's maintainers want to declare their inline types good enough for public consumption, then we should probably defer to them on that.

On the other hand, from the conversation in redis/redis-py#2738, it sounds like the redis maintainers acknowledge that their typing is pretty incomplete.

On the third hand, it also sounds from the conversation in redis/redis-py#2738 like they're unhappy with the quality of our stubs, and would prefer to provide their own type hints if possible.

I suppose the ideal situation would be if we contributed as many as possible of our types in typeshed to redis. But without knowing where the errors referenced in redis/redis-py#2738 (comment) are, it's hard to know where these would be welcome and where they wouldn't be.

@Avasam
Copy link
Collaborator

Avasam commented Sep 5, 2023

@AlexWaygood typeshed recently supports creating partial stubs on purpose. Exactly with cases like this in mind. It's something that is likely to happen eventually in pywin32 as well.

We can mark the stub as partial, but NOT stubtest incomplete. Then ignore known typed modules (with a comment) in stubtest-allowlist.
As more modules get typed in Redis, we can remove modules from types-Redis and keep bumping the version pin.

Type tests will of course need the base library to be installed (can't remember if they already do that. Pyright?)
Idk if it'll need a stubsabot update to prevent re-openning an issue.

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 5, 2023

Oh, so if we mark the stubs as partial, type checkers will only look at the annotations for the submodules we include in typeshed, and fall back to inline types in the runtime package for the submodules we don't include? (I'm not too familiar with the details of PEP-561!)

@Avasam
Copy link
Collaborator

Avasam commented Sep 6, 2023

This is the specification for partial stubs as per https://peps.python.org/pep-0561/#partial-stub-packages

[...] modules not found in the stub package SHOULD be searched for in parts four and five of the module resolution order above, namely inline packages and typeshed.

Type checkers should merge the stub package and runtime package or typeshed directories. This can be thought of as the functional equivalent of copying the stub package into the same directory as the corresponding runtime package [...]

Pyright's documentation about order resolution is relevant here as well: https://microsoft.github.io/pyright/#/import-resolution?id=resolution-order

@AlexWaygood AlexWaygood added the topic: redis Issues related to the redis third-party stubs label Sep 8, 2023
@meshantz
Copy link

meshantz commented Sep 8, 2023

Since there are so many upstream issues yet to be resolve, can we remove the recommendation to remove types-redis mentioned on the package page https://pypi.org/project/types-redis/:

Note: The redis package includes type annotations or type stubs since version 5.0.0. Please uninstall the types-redis package if you use this or a newer version.

Or if you don't want it removed, at least add some caveats?

@srittau
Copy link
Collaborator Author

srittau commented Sep 12, 2023

@meshantz Thanks for the suggestion. We now removed the wording and instead suggest du leave the package installed for now.

@Avasam
Copy link
Collaborator

Avasam commented Oct 14, 2023

Note that typeshed-internal/stub_uploader#108 will probably need to be fixed first to properly support partial type stubs for Redis.

@Avasam
Copy link
Collaborator

Avasam commented Mar 14, 2024

So, we're in march. Do we know which way we wanna go? I haven't checked the state of typing in redis in a while, but assuming it's still incomplete compared to typeshed, do we wanna keep going for a gradual transition?
redis/redis-py#2730 is still an issue when it comes to improving the typeshed redis stubs.

@sobolevn
Copy link
Member

It is incomplete, many types are just Anys, I vote for improving redis first and removing types-redis after that.

@srittau
Copy link
Collaborator Author

srittau commented Mar 15, 2024

I'm marking this issue as deferred for now.

@srittau
Copy link
Collaborator Author

srittau commented Jul 25, 2024

#12426 now marks redis as obsolete again. It's been nearly a year since the release of redis 5. While I didn't check the current state of the annotations in upstream redis, our current stubs only support redis 4 and there have been PRs to add redis 5 features. I think it's best if we continue to support the redis 4 stubs for another 6 months before removing them. Any typing improvements for redis 5 should be contributed to redis-py directly.

@srittau srittau changed the title Remove redis (not before March 2024) Remove redis (not before February 2025) Jul 25, 2024
@srittau srittau removed the status: deferred Issue or PR deferred until some precondition is fixed label Jul 25, 2024
@HassanAbouelela
Copy link
Contributor

HassanAbouelela commented Oct 11, 2024

@srittau

Not sure where best to ask this, but I don't think the typings here should be removed for v5.x. I've spent a very confused morning trying to figure out the state of typing for redis-py, but it seems like everything is still annotated with ResponseT even for the most basic methods, which is useless as typing:

https://github.com/redis/redis-py/blob/17db62e3c9ea796f5705d2857f49e52799057af7/redis/typing.py#L35

types-redis might need work to support 5.x, but it's far less work than annotating it in redis-py directly. I understand the reason why it was deprecated, but I don't see what an end-user is supposed to do here. Please advise if I've missed something, or if I should move this issue to redis-py directly.

See also: redis/redis-py#2399, redis/redis-py#2933

@srittau
Copy link
Collaborator Author

srittau commented Oct 14, 2024

@HassanAbouelela In the end this is a decision by the redis-py maintainers. As long as they commit to providing type annotations by providing a py.typed file, I don't want to step on their toes to do so. If redis-py were to drop the commitment and someone would step forward to update our stubs to redis-py 5, we could continue providing those stubs.

@HassanAbouelela
Copy link
Contributor

I understand, thank you.

@srittau
Copy link
Collaborator Author

srittau commented Nov 28, 2024

It seems that Redis Inc., owners of the Redis trademark are tightening their use of the trademark. (Cf. redis-rs/redis-rs#1419) As such I suggest we remove the stubs now, instead of February to avoid any potential legal issues. What should we do about the published packages on PyPI? Cc @ilevkivskyi, who "owns" the package (all typeshed package really) on PyPI as far as I know.

@Avasam
Copy link
Collaborator

Avasam commented Nov 28, 2024

I have opinions but IANAL. 😶

I have no issues dropping dropping support for the stubs earlier than intended (we already dragged their removal initially in hope of maintenance or cooperation with synchronizing the stubs with upstream, which didn't happen).

As for published package on PyPI, I would pity users using it if it has to be pulled, and I would hope for it not to be misused if transferred. Of course I would prefer that nothing happens with it. But that's outside my perogative.

@hauntsaninja
Copy link
Collaborator

Recognising that people in open source have complicated feelings about redis, I don't think there's much legal risk, especially so based on comments like redis-rs/redis-rs#1419 (comment)

I'm happy to mark the stubs as obsolete since upstream ships a py.typed, but I'd be strongly opposed to yanking releases or deleting the project or doing anything hasty that hurts our existing users.

srittau added a commit to srittau/typeshed that referenced this issue Nov 29, 2024
@srittau srittau mentioned this issue Nov 29, 2024
@srittau
Copy link
Collaborator Author

srittau commented Nov 29, 2024

#13157 removes the stubs from the repository. We can keep the PyPI project for now, unless we get complaints from Redis Inc.

@ilevkivskyi
Copy link
Member

Cc @ilevkivskyi, who "owns" the package (all typeshed package really) on PyPI as far as I know

Btw I don't really "own" typeshed_bot account, I think I am simply the only one of the people with password who added 2FA when it became mandatory. Anyway, I will be happy to help if something will be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: removal Pending removal of third-party distributions topic: redis Issues related to the redis third-party stubs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants