-
Notifications
You must be signed in to change notification settings - Fork 47
Restricts custom plot button to send to data explorer #3828
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
Conversation
jellybean2004
left a comment
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.
Seems to fix the issue.
Also tested in the invariant refactor branch, seems to do the expected job without causing any issues.
krzywon
left a comment
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.
My comments might be outside the scope of this PR, since the issues I'm noting are already present in main, but, since you're already working in this section, maybe address them?
| custom_action.triggered.connect(self.sendToDataExplorer) | ||
| self.insertAction(self.actions()[-1], custom_action) |
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.
Following the procedure outlined in #3827, when sending the residual plot to the data explorer, two data objects are created in the data explorer. Somewhere in lines 34-37, two identical actions are created, and both are triggered when the icon is clicked.
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.
It looks like this comes from the loop in sendToDataExplorer
search_name = self.parent.data
for item in search_name:
self.parent.manager.communicator.freezeDataNameSignal.emit(item.name)
In the parent data, the graph is listed twice. We may want to investigate further but given the nature of the bugs I'll leave it for this PR.
|
|
||
| # Re-enable after 3 seconds | ||
| QtCore.QTimer.singleShot(3000, lambda: self._actions["fitting"].setEnabled(True)) | ||
| QtCore.QTimer.singleShot(3000, lambda: self._actions["data_explorer"].setEnabled(True)) |
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.
I never had a chance to review the original PR. What is this 3 second button disable accomplishing?
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.
I'm afraid I don't know. I can't see any discussion about it in the original PR.
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.
I've removed this code as is appears to serve no purpose.
340ed59 to
4670153
Compare
Description
This PR restricts the functionality of the button known as "send to fitting" (now named "send to data explorer") to make sure it does not send data to perspectives, which has caused a number of bugs.
Fixes #3827
How Has This Been Tested?
Manually tested the procedure in #3827, the error does not occur. No data is sent to perspectives.
Review Checklist:
[if using the editor, use
[x]in place of[ ]to check a box]Documentation (check at least one)
Installers
Licensing (untick if necessary)