Skip to content
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

"Fill markers" button doesn't work in "Writing a custom viewer for glue with Qt" tutorial #2388

Closed
sergiopasra opened this issue Apr 16, 2023 · 3 comments · Fixed by #2389
Closed
Labels

Comments

@sergiopasra
Copy link
Contributor

sergiopasra commented Apr 16, 2023

Describe the bug
I'm following the "Writing a custom viewer for glue with Qt" tutorial. I have copied the TutorialViewer classes and viewer_state.ui. I load a csv file and I mostly obtain what is expected with one exception. The "Fill markers" toggle button does nothing. The fill property is never updated and the method _on_fill_changeis never called.

If I add a callback to self.checkbox, than I can get the desired behavior, but connect_checkable_button is not working correctly.

To Reproduce
Steps to reproduce the behavior such as:

  1. Copy the code in "Writing a custom viewer for glue with Qt"
  2. Load a CSV file
  3. Drag to plot
  4. Select "Tutorial Viewer"
  5. Click on "Fill markers"

Expected behavior
Markers on the plot are filled.

Details:

  • Operating System: Linux Fedora 37
  • Python version: Python 3.11.2, Python 3.10.10
  • Glue version: 1.9.2.dev1+g4717aa72
  • How you installed glue: pip
@Carifio24
Copy link
Member

Carifio24 commented Apr 17, 2023

Hi @sergiopasra, there is indeed a bug in the code given on the tutorial page. In line 132 of the code (inside the initializer for TutorialLayerStateWidget, replace

connect_checkable_button(self.layer_state, 'fill', self.checkbox)

with

self._connection = connect_checkable_button(self.layer_state, 'fill', self.checkbox)

The problem with the current code is that no reference to the created connection is retained, so it doesn't exist anymore when the callback property is modified. The containers that store the callback functions (echo.CallbackContainer) only store weak references to instance methods, and when the instance is garbage collected, the callback will be removed as well.

@sergiopasra
Copy link
Contributor Author

Thank you! Now code like this makes more sense:

self._connections = autoconnect_callbacks_to_qt(self.viewer_state, self.ui)

So the value returned by autoconnect_callbacks_to_qt is retained.

How do I store more than one callback? Looks like autoconnect_callbacks_to_qt returns a dictionary.

@Carifio24
Copy link
Member

It shouldn't really matter how they're stored - you could have a member value (of the TutorialLayerStateWidget) that's a list of connections, or a dictionary with the connections as values (like autoconnect_callbacks_to_qt), or just have each connection as a separate member value. As long as the connection is still retained somewhere, the callback setup will work correctly.

If you end up having a decent number of connections, note that you could also use the same pattern for the layer state as for the viewer state - create a .ui file that follows echo's autoconnect naming conventions and call autoconnect_callbacks_to_qt to create the connections for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants