-
Notifications
You must be signed in to change notification settings - Fork 33
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
1093- New Models useful for graphics section #1132
Conversation
…at need to be reflected in the UI
1d8ae7f
to
97ecf3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem you mentioned in chat was caused by the binding not keeping a reference to the source. This was a historical choice and I've changed it in nion-software/nionutils@f9ff65b.
I still show a couple tests failing.
Also, one minor change request I noted in the code: can you use double underscore for most private fields and callbacks? I recommend only using single underscore for protected fields, and even in that case I usually try not to expose the variable directly and instead provide a read-only property.
Example of method that will never be called in subclass and should be private (double underscore):
DisplayItemCalibrationValueModel._on_calibration_changed
-> DisplayItemCalibrationValueModel.__on_calibration_changed
Example of field that should not be used in subclasses and should be private:
TuplePropertyElementModel._listener
-> TuplePropertyElementModel.__listener
.
By default I would make almost all fields private unless there is a compelling reason that a subclass needs access.
Once you make the private fields, let me know if you want me to merge or want to keep adding to this PR.
NOTE: you will want to merge from the main branch before pushing another PR, to get the numpy 2 changes, which are required to utilize the new nionutil changes. Or rebase and force push. |
Should be all good now. If possible I would like to get this one merged quickly as it will be useful with the Graphics Inspector section that I am currently working on. |
Squashed and merged here f3e67d8 Thanks! This looks like a good approach going forward (using models instead of custom bindings). We may want to move |
@cmeyer the reason I put element in there was because the model was to access an element of the tuple property rather than the whole tuple. |
Wouldn't it be called |
This is to add 2 new models that I am finding will be useful for the graphics section of the inspector work:
TuplePropertyElementModel - many of the graphics sizes and positions are stored in tuples, this model is to access a single element of that tuple and to update the tuple when the value is changed.
DisplayItemCalibratedValueModel - the sizes and positions of the graphics can be displayed in a number of different units and this model watched for the calibration to be changed to update the values in the UI