Skip to content

Conversation

@johnfraney
Copy link
Contributor

@johnfraney johnfraney commented Jan 20, 2025

Adds initial types and type-checking with Pyright. This contributes to #468 but doesn't finish it.

Notes

There's still a lot of typing to do. I didn't want to introduce any friction to other development, so I didn't add a py.typed file or type-checking in a pre-commit hook or CI. Those are things we can do once we have solid types.

There are times when I'm unsure whether we should be using str or str | bytes/AnyStr, and I tried to get that right through how the variables are used. I hope I got them right.

There are quite a few missing imports in src/textblob/en/__init__.py: https://github.com/johnfraney/TextBlob/blob/dev/src/textblob/en/__init__.py#L1, but that seemed a little out-of-scope for this PR.

Results

Pyright results before:

$ pyright src
[...]
178 errors, 3 warnings, 0 informations

After:

$ pyright
[...]
129 errors, 1 warnings, 0 informations

@johnfraney johnfraney force-pushed the initial-typing branch 5 times, most recently from 813a787 to 2e653e3 Compare January 20, 2025 14:51
Copy link
Owner

@sloria sloria left a comment

Choose a reason for hiding this comment

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

thanks! looks good on a quick skim 👍

we should add type checking to tox/CI as well, but this can happen in a follow-up PR. I agree this can be an incremental effort. here's how it's set up in marshmallow (with mypy but easy enough to swap it out with pyright

https://github.com/marshmallow-code/marshmallow/blob/944f6de1b7528d87f190e20f132348be360b6e3c/tox.ini#L13-L17

https://github.com/marshmallow-code/marshmallow/blob/944f6de1b7528d87f190e20f132348be360b6e3c/.github/workflows/build-release.yml#L28

@johnfraney johnfraney force-pushed the initial-typing branch 2 times, most recently from edfbb7b to 600f255 Compare January 20, 2025 18:25
@sloria sloria merged commit 5765091 into sloria:dev Jan 20, 2025
7 checks passed
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.

2 participants