typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

Remove redis (not before February 2025)

Open srittau opened this issue 2 years ago • 15 comments


~~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 avatar Aug 16 '23 07:08 srittau

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.

srittau avatar Aug 21 '23 09:08 srittau

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 https://github.com/redis/redis-py/pull/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 https://github.com/redis/redis-py/pull/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 https://github.com/redis/redis-py/pull/2738#issuecomment-1541586541 are, it's hard to know where these would be welcome and where they wouldn't be.

AlexWaygood avatar Aug 21 '23 12:08 AlexWaygood

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

Avasam avatar Sep 05 '23 16:09 Avasam

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

AlexWaygood avatar Sep 05 '23 16:09 AlexWaygood

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

Avasam avatar Sep 06 '23 18:09 Avasam

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?

meshantz avatar Sep 08 '23 17:09 meshantz

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

srittau avatar Sep 12 '23 15:09 srittau

Note that https://github.com/typeshed-internal/stub_uploader/issues/108 will probably need to be fixed first to properly support partial type stubs for Redis.

Avasam avatar Oct 14 '23 16:10 Avasam

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? https://github.com/redis/redis-py/issues/2730 is still an issue when it comes to improving the typeshed redis stubs.

Avasam avatar Mar 14 '24 19:03 Avasam

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

sobolevn avatar Mar 15 '24 10:03 sobolevn

I'm marking this issue as deferred for now.

srittau avatar Mar 15 '24 14:03 srittau

#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 avatar Jul 25 '24 11:07 srittau

@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

HassanAbouelela avatar Oct 11 '24 10:10 HassanAbouelela

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

srittau avatar Oct 14 '24 08:10 srittau

I understand, thank you.

HassanAbouelela avatar Oct 14 '24 17:10 HassanAbouelela

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.

srittau avatar Nov 28 '24 20:11 srittau

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.

Avasam avatar Nov 28 '24 23:11 Avasam

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 https://github.com/redis-rs/redis-rs/issues/1419#issuecomment-2503578646

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.

hauntsaninja avatar Nov 29 '24 09:11 hauntsaninja

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

srittau avatar Nov 29 '24 13:11 srittau

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.

ilevkivskyi avatar Dec 02 '24 00:12 ilevkivskyi