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

Replace type checks with type guard #120

Open
davidhopkinson26 opened this issue Jul 13, 2023 · 4 comments
Open

Replace type checks with type guard #120

davidhopkinson26 opened this issue Jul 13, 2023 · 4 comments
Labels
CI improvements to continuous integration

Comments

@davidhopkinson26
Copy link
Contributor

Tubular currently has a lot of type checking. This could be replaced with typeguard to simplify the code and reduce test burden.

@davidhopkinson26 davidhopkinson26 added the CI improvements to continuous integration label Jul 13, 2023
@davidhopkinson26
Copy link
Contributor Author

Closely linked with #116 as type hints required for typeguard

@davidhopkinson26
Copy link
Contributor Author

the downside of this would be that we would no longer get the class specific error messages prefixed by self.classname(). This is quite a useful feature when running transformers in a long pipeline so may be a good justification for keeping the current error handling.

@davidhopkinson26
Copy link
Contributor Author

question for investigation - Can we extend typeguard or something similar to check types of columns in pandas dataframes?

@adamsardar
Copy link
Contributor

adamsardar commented Jan 2, 2024

Now that #118 has been cracked, this ticket comes into focus.

The other big player in the argument validation world is pydantic and the @validate_call decorator. Comparing the two:

Pydantic

  • ✔️ Technically a parsing library, not a validation library (see here). It makes no guarantee whatsoever about checking the form of data you give to it; instead it makes a guarantee about the form of data you get out of it.
  • ✔️ Makes more use of Annotated field information; we can stipulate that an argument needs to be positive definite concisely and integrate with static type checkers. This is a nicer user experience.
  • ✔️ We already use pydantic in our stack internally, so the effective dependency change is low
  • ❌ Errors are raised as a ValidationError, not a TypeError as is currently done in the codebase. This would be a breaking change unfortunately.
  • ❌ It's just a decorator - there's no smart way to add it across a module without annotating every method individually.
  • ❌ validate_arguments decorator is in beta.

Typeguard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI improvements to continuous integration
Projects
None yet
Development

No branches or pull requests

2 participants