magicgui icon indicating copy to clipboard operation
magicgui copied to clipboard

Nested pydantic model containers

Open ndxmrb opened this issue 1 year ago • 2 comments

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:

  • Proper test e.g. for getting nested values from ValueWidgets and ContainerWidgets - as I didn't really know where to start
  • QGroupBox option as mentioned in #317

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 avatar Sep 06 '24 12:09 ndxmrb

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

ndxmrb avatar Sep 06 '24 13:09 ndxmrb

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). Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
src/magicgui/schema/_ui_field.py 36.36% 7 Missing :warning:
src/magicgui/widgets/bases/_container_widget.py 85.71% 1 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Sep 10 '24 09:09 codecov[bot]

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 :)

tlambert03 avatar Nov 22 '24 14:11 tlambert03

Sorry for the silence as well @tlambert03. I'm still willing to work on it. I think the latest work here has indeed helped with the list types. But unfortunately, I'm not sure anymore what problem exactly I encountered. Either way, I can now use a nested pydantic model alongside a list field and validate the resulting dict without any issues.

Do you think the NestedValueWidgets type is still necessary, now that containers subclass ValueWidgets?

I'll update this (or make a new PR, if that's easier) as soon as I know how to go on with that.

ndxmrb avatar Apr 16 '25 12:04 ndxmrb

Closing in favor of #704.

ndxmrb avatar Jun 19 '25 16:06 ndxmrb