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.

Sign up for GitHub

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

Jump to bottom

Add missing type hints #1321

Closed
alifeee opened this issue Oct 19, 2023 · 5 comments
Closed

Add missing type hints #1321

alifeee opened this issue Oct 19, 2023 · 5 comments
Assignees
@lavigne958
Labels
good first issue Improvement
Milestone
6.0.0

Comments

@alifeee
Copy link
Collaborator

alifeee commented Oct 19, 2023 •
edited
Loading

This work should be done in the 6.0.0 release branch

Run mypy --install-types --non-interactive --ignore-missing-imports ./gspread (from CI) to see which types are broken/missing
@alifeee alifeee mentioned this issue Oct 19, 2023
Closed
@alifeee alifeee added this to the 6.0.0 milestone Oct 19, 2023
@alifeee alifeee mentioned this issue Oct 31, 2023
Merged
@bpshaver
Copy link

bpshaver commented Oct 31, 2023 •
edited
Loading

Looking into this. Most of the type errors come from one place.

In utils.py, mypy doesn't like that the named tuple singleton instances are created at the same time as the named tuple class is created.

Dimension = namedtuple("Dimension", ["rows", "cols"])("ROWS", "COLUMNS")

It seems like a better solution for this is a StrEnum. I'll see about making the change and open a PR.

@alifeee
Copy link
Collaborator Author

alifeee commented Oct 31, 2023

Hi! Thanks for looking into it!

One thing I forgot to mention in the issue: this work should be done on the 6.0.0 release branch. On this branch, as you suggested, we have already switched to StrEnums.

Also, see the linked PR #1337. I (think that I) fixed the remaining type issues there, and the work is complete. If you are still interested, do have a look. Some things may be patched over with Any that could be done differently, etc.

@bpshaver
Copy link

bpshaver commented Oct 31, 2023 •
edited
Loading

Well drats. I've already made all the necessary changes and was about to open a PR. Serves me right for skimming this issue.

bpshaver added a commit to bpshaver/gspread that referenced this issue Oct 31, 2023
@bpshaver
refactor(types): closes burnash#1321
932a0b5 
Replaces singleton named tuples in utils.py with StrEnums. Adds the qualified dependency for str enum to pyproject.toml for python versions below 3.11. Fixes one or two other minor type issues.

Signed-off-by: Ben Shaver <benpshaver@gmail.com>
@bpshaver
Copy link

bpshaver commented Oct 31, 2023 •
edited
Loading

Consider #1340 a suggestion. That way you only install strenum if the python version is below 3.11.

@alifeee
Copy link
Collaborator Author

alifeee commented Nov 1, 2023

closed by #1337

Well drats. I've already made all the necessary changes and was about to open a PR. Serves me right for skimming this issue.

My bad for not including the branch in the issue description. I should have done this, especially as I tagged this as good first issue

@alifeee alifeee closed this as completed Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lavigne958 lavigne958

Labels
good first issue Improvement
Projects
None yet
Milestone
6.0.0
Development

When branches are created from issues, their pull requests are automatically linked.

1321-add-missing-type-hints burnash/gspread
3 participants
@bpshaver @alifeee @lavigne958