Skip to content

Conversation

@ndxmrb
Copy link

@ndxmrb ndxmrb commented Jun 19, 2025

Hi @tlambert03,
restarting my work from #669 after the ValuedContainer changes.

Still, some things are missing:

Tried my best with the types again, hope it's okay that way.

Thanks for having a look!

@codecov
Copy link

codecov bot commented Jun 19, 2025

Codecov Report

❌ Patch coverage is 94.17476% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.32%. Comparing base (17ec0d8) to head (89f2972).

Files with missing lines Patch % Lines
src/magicgui/schema/_ui_field.py 85.71% 4 Missing ⚠️
src/magicgui/widgets/_concrete.py 96.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #704      +/-   ##
==========================================
+ Coverage   89.27%   89.32%   +0.04%     
==========================================
  Files          40       40              
  Lines        4820     4889      +69     
==========================================
+ Hits         4303     4367      +64     
- Misses        517      522       +5     

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

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

Copy link
Member

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

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

hey @ndxmrb

so sorry for the long delay. couple concerns about typing and error catching. we don't need to belabor them too much, if you can fix great, otherwise let me know.

@tlambert03 tlambert03 mentioned this pull request Dec 11, 2025
@tlambert03
Copy link
Member

@ndxmrb, I made some significant changes to this PR. The main thing that was holding me back here was the return type of create_widget (up here) ... it broke the clean pattern of having all UIFields returning a widget where you could be certain to query .value ... (though i recognize you were doing the least changes necessary to get the desired functionality). I made a proper ModelContainerWidget, which is essentially the concrete version of a ValuedContainerWidget (and is a widget specifically made for dataclass-like objects). It implements the proper interface to retain consistency with the previous pattern. Please have a last look if you're able/willing.

I know it's been so long so you might have forgotten/lost interest by now. So I'll give this a week or so before merging

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