-
Notifications
You must be signed in to change notification settings - Fork 52
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
Rhfogh develop Adapt to newest paramsgui changes (from jbf) #819
Merged
Conversation
This file contains 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
rhfogh
changed the title
Rhfogh develop Adapt to newest paramsgui cahnes (from jbf)
Rhfogh develop Adapt to newest paramsgui changes (from jbf)
Nov 20, 2023
rhfogh
force-pushed
the
rhfogh_develop
branch
2 times, most recently
from
November 23, 2023 10:49
d280499
to
6cbc53e
Compare
@rhfogh I think we should remove wf_type field |
No problem - it will take a few experiments before we settle all of this. I'll just remove it again ASAP. |
The wf_type field has now been removed |
rhfogh
force-pushed
the
rhfogh_develop
branch
from
December 12, 2023 11:37
f6b249a
to
60d131a
Compare
Some general clean-up
Also WIP started to change signal from GUI to core to a function call
rhfogh
force-pushed
the
rhfogh_develop
branch
from
December 12, 2023 15:21
60d131a
to
63c887b
Compare
jbflo
approved these changes
Dec 12, 2023
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.
This PR (and the companion PR in mxcubeqt) means that the qt UI now works with the latest mxcubecore changes (and includes some minor clean-ups and fixes).
A couple of points need consideration, for the changes to the paramsgui:
The entries for new wf_type field has been changed as it did not work with the system; this raises a general point:
As the system works, the schema["properties"] contains an entry for every data field, but none for pure container fields.
The ui_schema, on the other hand, contains only entries for displayed fields, as a nested structure of containers and contents.
It is therefore not possible to add parameters in the ui_schema for fields that are not displayed, since these entries do not appear in the nested container structure. Also the wf_type field, as entered, was displayed on top of the main container field, proving that all displayed fields should be inside a container. As the system works after this PR, all fields that do not appear in the nested hierarchy of containers (including wf_type) are automatically treated as hidden and read_only, but can be accessed through the general dictionary of parameter values.
If this is problematic for the web implementation we need to consider alternatives together.
I should add that the organisation of ui_schema could be changed if we have a good reason for wanting to do so, but this is the way it is at the moment and that was the quickest way of making the system functional.
I have replaced the signal sent from the GUI with a call to a function on the mxcubecore side (which then in turn emits the signal). To be generic this function had to be put on a fairly general object on the mxcubecore side. I opted for the Beamline object as a neutral location. The function, 'emit', is almost identical to the emit function in HardwareObjectMixin, but Beamline is not a subclass of this class, as the beamline, while yml-configured, is not a hardware object. 1) I am not sure of the exact considerations with what should be imported from core to GUI - might we want a different location? 2) it would be neater if we could avoid the semi-duplication of the emit function - does anyone have an idea?