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

Upgrade typing for tests (via pyupgrade) #287

Merged
merged 1 commit into from
Oct 15, 2023

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Oct 3, 2023

This was mostly automated with pyupgrade. Review appreciated as I'm a typing novice.

Note that these files have from __future__ import annotations at the top; xref #273

@adamjstewart
Copy link
Collaborator

FWIW, I stopped using from __future__ import annotations in my code because it hasn't yet been adopted into the official standard and it doesn't seem like they're close to reaching a decision: https://docs.python.org/3/library/__future__.html

Downside is that the updated type hints require Python 3.10 to run mypy. Upside is they're much prettier. We could also consider dropping support for older Python, but I don't see any reason to rush that.

Basically, I'm not opposed to this change, but I also don't think it's necessary. As long as we're consistent across all files I'm happy.

@mwtoews
Copy link
Member Author

mwtoews commented Oct 15, 2023

Thanks for your incite @adamjstewart ! I'll merge for now, since the scope is just for tests. It was also part of some pre-commit scoping that I'm considering to add.

@mwtoews mwtoews merged commit ce4c8d0 into Toblerity:master Oct 15, 2023
37 checks passed
@mwtoews mwtoews deleted the typing-upgrade branch October 15, 2023 02:24
JDBetteridge pushed a commit to JDBetteridge/rtree that referenced this pull request Nov 1, 2023
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.

None yet

2 participants