-
Notifications
You must be signed in to change notification settings - Fork 13
refactor: pull in just the model from 451 #453
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #453 +/- ##
==========================================
+ Coverage 87.39% 87.43% +0.04%
==========================================
Files 100 105 +5
Lines 11122 11472 +350
==========================================
+ Hits 9720 10031 +311
- Misses 1402 1441 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…handling and add unit test for StandardIcon
…test for set_channel_group functionality
Lol, yes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#452 has gotten massive, since it has gain a lot of features. So this PR is just to pull out some of the code that kind of stands on its own, to test it in isolation, before returning to #452.
The biggest change here is that I've decided to use our own internal python model for config groups, rather than using the dataclasses provided by
pymmcore_plus.models
. They are largely the same (though these are based on pydantic rather than dataclasses), but I found myself wanting to modify the models too much and I don't want to do the two-repo game. This does mean that some of the widgets here (namely, the hardware wizard) use the model over there, and some will use the internal one... but that will be temporary. Both can coexist, though it does add to confusion and should be unified again in the near future. (I've learned so much on the QAbstractItemModel thing here that I will want to bring it back over to the hardware wizard again anyway).It also updates the icon pattern to use StandardIcon
This PR lets me focus on what needs testing for the changes, before merging back into 452