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

Add support for type casting in the typed decorator #635

Merged
merged 6 commits into from
Oct 9, 2024

Conversation

ncoop57
Copy link

@ncoop57 ncoop57 commented Oct 8, 2024

This PR adds support for the typed decorator to perform automatic casting when the cast flag is set to True. It maintains the current API for backwards compatibility so that users can now specify the following:

@typed
def discount(price:int, pct:float) -> float:
    return (1-pct) * price

with ExceptionExpected(TypeError): discount(100.0, .1)
# or

@typed(cast=True)
def discount(price:int, pct:float) -> float:
    return (1-pct) * price

assert 90.0 == discount(100.5, .1) # will auto cast the float "100.5" to the int "100"

The conversions are handled by a type_map dictionary that maps the expected type (i.e. the one specified as a type hint)
to the function to perform the conversion.

This allows for better validation handling for libraries like FastHTML instead of immediately throwing an error for common cases that would be expected to be properly handled like the str "on" to be convert to the int 1 or the str "none" to be converted to the int 0.

TODOs before merging:

  • Add better type conversion functions for the type_map that handles common cases such as converting the string "True" to the bool True if the expected type is a bool and the given one is a str.
  • Handle custom data types
  • Handle optional types (e.g., a: int:None
  • Handle container types like list[str] or set[int] Not a priority. Can revisit if it is needed.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jph00
Copy link
Member

jph00 commented Oct 8, 2024

lgtm @ncoop57 ! And yeah for the typemap values, just use the various str2... functions we have, moving them from fasthtml into here as needed (and updating fasthtml to use them, and updating its settings.ini fastcore dep). Sound OK?

@ncoop57 ncoop57 marked this pull request as ready for review October 9, 2024 02:17
@ncoop57
Copy link
Author

ncoop57 commented Oct 9, 2024

@jph00 Okay, I think the PR is ready for a full review!

@jph00
Copy link
Member

jph00 commented Oct 9, 2024

You got some conflicts @ncoop57 !

@ncoop57
Copy link
Author

ncoop57 commented Oct 9, 2024

@jph00 whoops! Should be good now 🤓

@ncoop57
Copy link
Author

ncoop57 commented Oct 9, 2024

@jph00 hold up on merging!

@ncoop57
Copy link
Author

ncoop57 commented Oct 9, 2024

Looking at an error when previewing the docs locally

@ncoop57
Copy link
Author

ncoop57 commented Oct 9, 2024

@jph00 okay, fixed the issue I had previewing the docs locally and made some slight tweaks by hiding some additional tests that I don't think need to be in the docs as it didn't look that great when previewing. Lmk what you think!

@jph00
Copy link
Member

jph00 commented Oct 9, 2024

lgtm!

@jph00 jph00 merged commit f4a05cf into fastai:master Oct 9, 2024
12 checks passed
@jph00 jph00 added the enhancement New feature or request label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants