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.

Sign up for GitHub

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

Jump to bottom

Support TypedDict field as Dict[str, Any] #237

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support TypedDict field as Dict[str, Any] #237

wants to merge 1 commit into from

Conversation

isra17
Copy link
Collaborator

@isra17 isra17 commented Nov 11, 2022

Title says it all. Nothing to fancy, just a simple mapping of TypedDict -> Dict[str, Any]

@isra17 isra17 self-assigned this Nov 11, 2022
@codecov
Copy link

codecov bot commented Nov 11, 2022 •
edited
Loading

Codecov Report

Merging #237 (59519a1) into main (0c43ff5) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #237   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          472       499   +27     
  Branches        90        98    +8     
=========================================
+ Hits           472       499   +27
Impacted Files Coverage Δ
src/desert/_make.py 100.00% <100.00%> (ø)
tests/test_make.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@isra17 isra17 force-pushed the isra/support-typeddict branch 7 times, most recently from f6f7a7a to 62f510a Compare November 17, 2022 14:36
@isra17
Copy link
Collaborator Author

isra17 commented Nov 17, 2022

Moved to typing_inspect instead of homemade is_typeddict

@isra17
Copy link
Collaborator Author

isra17 commented Nov 17, 2022

Look like 'inspect_typing' has a bug 😞

@isra17 isra17 force-pushed the isra/support-typeddict branch 3 times, most recently from 75750d5 to 7965492 Compare November 17, 2022 16:09
@isra17
Copy link
Collaborator Author

isra17 commented Nov 17, 2022

It will require #238 to fix the coverage issue.

@isra17 isra17 force-pushed the isra/support-typeddict branch 2 times, most recently from 94308c3 to 82d0984 Compare November 18, 2022 15:32
altendky
altendky requested changes Nov 19, 2022
View reviewed changes
Copy link
Member

@altendky altendky left a comment

Choose a reason for hiding this comment

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

I'd think it would be nice to support the hints provided by a TypedDict, but this seems a relevant step in that direction and useful on its own. Thanks.

There are a few tweaks I think I'd like. If they explode in your face and make a mess, let me know and we can move forward as is knowing that I may make an attempt at the changes myself after this.

tests/test_make.py Outdated Show resolved Hide resolved
src/desert/_make.py Outdated Show resolved Hide resolved
@isra17 isra17 force-pushed the isra/support-typeddict branch 2 times, most recently from de8324f to 5ce23ac Compare November 27, 2022 21:44
@isra17
Copy link
Collaborator Author

isra17 commented Nov 27, 2022

I addressed your comments! Thanks for the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@altendky altendky altendky requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@isra17 isra17

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@isra17 @altendky