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

Add typings to the redis.Redis class #3252

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bitterteriyaki
Copy link

@bitterteriyaki bitterteriyaki commented May 28, 2024

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

NOTE: This PR adds breaking changes.

This PR aims to add static typings to the redis.client.Redis class. This is still a work in progress and this PR must not be merged yet. Please give me any feedback on that.

@bitterteriyaki bitterteriyaki marked this pull request as draft May 28, 2024 03:51
@bitterteriyaki
Copy link
Author

In some parameters, I don't know what is the proper type. I'll fix these later.

@gerzse
Copy link
Contributor

gerzse commented May 28, 2024

Hi @bitterteriyaki, thanks for your desire to contribute!

Currently redis-py supports Python 3.8 up to and including 3.11, and support for 3.12 is planned.

In general it's best to use language constructs that work in all these versions. For example Union[str, None] instead of str | None, which is available only since 3.10. The Self construct can be made available in versions earlier than 3.11?

@bitterteriyaki
Copy link
Author

bitterteriyaki commented May 28, 2024

Currently redis-py supports Python 3.8 up to and including 3.11, and support for 3.12 is planned.

Hi, @gerzse. Thank you for the answer. Since Python 3.7 was dropped, It would be nice to update the documentation.

In general it's best to use language constructs that work in all these versions. For example Union[str, None] instead of str | None, which is available only since 3.10.

Alright. I'll be changing this.

The Self construct can be made available in versions earlier than 3.11?

According to the typing documentation, no. I'll also be changing this.

@bitterteriyaki
Copy link
Author

Instead of typing.Self, I am using from __future__ import annotations which is available for old Python versions and is a library for "future" which is not available for these versions.

redis/client.py Show resolved Hide resolved
redis/client.py Show resolved Hide resolved
redis/client.py Show resolved Hide resolved
@Vulwsztyn Vulwsztyn mentioned this pull request Aug 19, 2024
6 tasks
@bitterteriyaki
Copy link
Author

bitterteriyaki commented Aug 20, 2024

Hi, @Vulwsztyn. This PR is still a draft and it is not complete. Review once I finish it, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants