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

Nested pydantic model containers #669

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ndxmrb
Copy link

@ndxmrb ndxmrb commented Sep 6, 2024

Hi @tlambert03,
when using nested pydantic models, I found this post on how to add a simple way of nesting widgets/containers. When trying to use the values, I needed to modify asdict() as well. Although there might be a reason you didn't just implement it already when answering the post (probably due to #317), I thought I'd make a pull request just in case.

Notably, some things are missing:

Also, I tried my best with the types, but I probably don't oversee the grand scheme of things... 😅

I hope it's helpful, but I'm aware it might not be, so feel free to just close 😄

Thanks for having a look!

@ndxmrb
Copy link
Author

ndxmrb commented Sep 6, 2024

I just saw that this probably messes with List types. Set to draft for now.

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Project coverage is 89.03%. Comparing base (379bc62) to head (15263c6).

Files with missing lines Patch % Lines
src/magicgui/schema/_ui_field.py 36.36% 7 Missing ⚠️
src/magicgui/widgets/bases/_container_widget.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #669      +/-   ##
==========================================
- Coverage   89.16%   89.03%   -0.14%     
==========================================
  Files          39       39              
  Lines        4746     4761      +15     
==========================================
+ Hits         4232     4239       +7     
- Misses        514      522       +8     

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

@tlambert03
Copy link
Member

sorry for the radio silence here @ndxmrb. This would be a great addition if you want to keep banging at it.

I did indeed see the challenge with list types. I wonder whether #663 might have helped the situation here (I haven't had time to look closely, so it's possibly no). @hanjinliu may also have some insights...

if you have a moment and want to resolve merge conflicts and see what the state of things look like now, that would be great :)

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.

2 participants