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

feat: Support native inference parser for dataclass-like annotations. #141

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

DanCardin
Copy link
Owner

@DanCardin DanCardin commented Aug 13, 2024

This is probably blocked on #117. Realistically, the current annotation and extra_annotations used by the parsers is bad DX in comparison to something like TypeView. If all the parsers are going to end up refactored to be defined in terms of single-level value/annotation, it's going to make a lot more sense when the actual annotation bit is a TypeView instead.

Result based of discussion from #137

The main problem with automatic dataclass-like inference, is that we can't know, without the annotation and an explicit decision from the user, whether a list[T] input is the appropriate input for the class itself, or if it wants the inputs to be splatted into the constructor.

For anything where the single value would make sense today, it should already work.


As implemented, this is perhaps a decent amount more complicated because it's using the invoke machinery to apply all parse operations now. It's not necessarily a problem, it's just a decent amount more code being evaluated for every parameter to introspect the function's parameter annotations before calling the function, versus just calling the function.

It's now moderately more complicated at the collection phase, but not overly so, and the actual parse step is no more complicated. And compose can be handled with lists of parsers, which is the biggest simplification.

Base automatically changed from dc/parse to main August 13, 2024 21:15
@DanCardin DanCardin force-pushed the parse-option-for-dataclasses branch 2 times, most recently from 3eb3b71 to a938004 Compare October 6, 2024 12:04
Copy link

codecov bot commented Oct 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f9f5e0e) to head (f932943).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #141   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         2159      2188   +29     
  Branches       475       474    -1     
=========================================
+ Hits          2159      2188   +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DanCardin DanCardin force-pushed the parse-option-for-dataclasses branch 3 times, most recently from 0eed075 to 2691124 Compare October 8, 2024 14:59
@DanCardin DanCardin marked this pull request as ready for review October 8, 2024 21:25
@DanCardin DanCardin merged commit 6d2f1c5 into main Oct 9, 2024
10 checks passed
@DanCardin DanCardin deleted the parse-option-for-dataclasses branch October 9, 2024 23:42
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.

1 participant