Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(types): improve typing #720

Merged
merged 4 commits into from
Jul 12, 2023
Merged

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Jun 23, 2023

This fixes an issue with os.PathLike[str] being accepted but not present in the typing. It also uses collections.abc wherever possible instead of typing (can't fully change all of them until 3.9+ is supported, due to runtime requirements). It also uses Sequence/Mapping/Iterable for input parameters (you should be generic in input, specific in output) - doing so adds a few copy functions and improves safely a bit, fewer arguments are allowed to be mutated later this way). A few TypeVars were added - they can be replaced with native syntax in Python 3.12+.

Noticed in scikit-build/scikit-build-core#408 where mypy found two real problems in my noxfile, and one false positive, which this fixes.

@henryiii henryiii force-pushed the henryiii/fix/types branch 2 times, most recently from c1de963 to d430b4b Compare June 23, 2023 21:19
This fixes an issue with os.PathLike[str] being accepted but not present in the typing. It also uses collections.abc wherever possible instead of typing (can't fully change all of them until 3.9+ is supported, due to runtime requirements). It also uses Sequence/Mapping/Iterable for input parameters (you should be generic in input, specific in output) - doing so adds a few copy functions and improves safety a bit, fewer arguments are allowed to be mutated later this way). A few TypeVars were added - they can be replaced with native syntax in Python 3.12+.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/fix/types branch from d430b4b to 160c47f Compare June 23, 2023 21:42
Copy link
Collaborator

@theacodes theacodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@theacodes
Copy link
Collaborator

Patched up some conflicts, waiting for CI to merge

@theacodes theacodes merged commit f46e46e into wntrblm:main Jul 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants