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

Complete type annotations #227

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented May 20, 2024

Closes #214

This changeset adds a bunch of missing annotations and other fixes/workarounds to get routee-compass to a complete set of annotations (i.e. mypy --strict can be run over the whole wrapper with no errors). This also updates and re-enables the type-checking step of the test workflow (and adds the necessary type stubs to the [dev] extra)

@SnoopJ SnoopJ force-pushed the typing/gh214_finish-wrapper-types branch from 8eb3b90 to 55d9801 Compare May 20, 2024 21:01
@SnoopJ SnoopJ force-pushed the typing/gh214_finish-wrapper-types branch from 55d9801 to 2c27d60 Compare May 21, 2024 04:22
@nreinicke
Copy link
Collaborator

Thanks for adding all of these in! We made a couple of changes when sprinting today that will cause a couple merge conflicts here (sorry!)

We also decided to make the geospatial-like dependencies optional and then just rely on the TYPE_CHECKING parameter and en-quoting the optional types.

@SnoopJ
Copy link
Contributor Author

SnoopJ commented May 21, 2024

No trouble at all on the merge conflicts, I'll sort them out. Guarding/quoting the geospatial stuff sounds pretty straightforward to take care of.

It turned out that I didn't have much time today, but I did get a few moments to look at the problems with the annotation of the agg parameter. The core problem is I believe the conflict between Union[VeryLongCallableT, overloaded function] and Optional[VeryLongCallableT], but I'm not entirely sure what the overloaded bit is about.

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.

Finish adding mypy type checking
2 participants