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

Update viewer variable when changing focused console (multiple consoles sharing the same kernel handling) #42

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Dec 9, 2024

Closes #40
Closes #41 (superseded)

Follow up of the idea at #40 (comment)

Example script:

import napari
v1 = napari.Viewer()
v2 = napari.Viewer()
napari.run()

Run via python script.py

A preview:

console_viewer_reference_update

@dalthviz dalthviz changed the title Update viewer variable when changing focused console (multiple consoles sharing the same kernel) Update viewer variable when changing focused console (multiple consoles sharing the same kernel handling) Dec 9, 2024
@Czaki
Copy link
Contributor

Czaki commented Dec 10, 2024

looks good

@jni
Copy link
Member

jni commented Dec 11, 2024

So, I think this is better than the status quo so I'm happy for it to go in. However, now that we have a full namespace capture, I don't think we need to inject viewer at all, and we should just drop that special case. (Assuming the env capture is working with viewers, I don't know what the deal was with @psobolewskiPhD's testing, seems to have magically resolved itself???)

The viewer special variable was always a hack around not capturing the full namespace, and would now break something like:

import napari

viewer = napari.Viewer(title='viewer 1')
viewer2 = napari.Viewer(title='viewer 2')

napari.run()

Now, we can't access viewer 1 if we are in viewer 2, which is a problem.

I know it's a big change to drop the special variable, but we could do so gradually. Suggestions (can be taken individually, or simply ignored 😅):

  • only add viewer if there is not already a variable viewer in the namespace. That is, never shadow an existing name.
  • rename the special variable to this_viewer. This makes a lot of sense to me with the behaviour in this PR.
  • (combo option): if there is already viewer in the namespace, but not this_viewer, use this_viewer, otherwise, add viewer to preserve backwards compatibility.

What do folks think?

@psobolewskiPhD
Copy link
Member

My issue appears to be related to pyqt6: #40 (comment)

Something I noticed:

import napari

napari.Viewer(title="v1")
v2 = napari.Viewer(title="v2")
napari.run()

The console button in v1 was toggling the console of v2! SO things are quite entangled now.

Also I think every snippet we have anywhere does viewer = napari.Viewer() so the example @jni gave is legit.

Honestly, I didn't even know about the magic viewer until now. I thought it was a thing in the case of launching napari from the command line.
I agree with @jni my thinking is:

  1. if user provides viewer names, e.g. v1, v2, use those--this goes for viewer too--so no magic viewer. I don't think anyone knows about the magic viewer nor relies on it outside the case of launch from terminal, so we can drop it here. If we want a new this_viewer, I'm ok with it, but not important--noone will know about it 🤣
  2. if there is no viewer name, like launch from terminal or my script above, etc. use the magic viewer. What about more than one unnamed viewer? like someone using napari.Viewer() multiple times? Then I guess we need to append a number like the other PR? Although in this specific case, with the user not specifying anything we could do the each viewer is the magic viewer`` as in this PR and assign viewer1, `viewer2` etc. to keep it straight.

@Czaki
Copy link
Contributor

Czaki commented Dec 11, 2024

What do folks think?

I do not agree. Some plugins may spawn the next viewer instance (ex PartSeg allows for this) so you may have more than one viewer without script.

@dalthviz
Copy link
Member Author

What if instead of adding a viewer variable a function to get the viewer is added? Maybe something like a get_viewer or a get_console_viewer function? From a console you could do then something like:

imagen

The function would always return the viewer related with the console you are using

@jni
Copy link
Member

jni commented Dec 13, 2024

We do already have napari.current_viewer, so we can just expose that.

@dalthviz
Copy link
Member Author

Another idea, in case the main need is to be able to access any viewer from any console (and even if no variable is assigned to the viewer when creating it), is to create a get_viewers function. I think somewhere I saw that a weak reference to all the available viewers is kept so maybe exposing that could be an alternative to pushing a viewer variable or any other variable that could potencially shadow an already defined user variable? 🤔

@Czaki
Copy link
Contributor

Czaki commented Dec 13, 2024

I think that you are referring to napari/napari#5664

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Dec 14, 2024

What do folks think?

I do not agree. Some plugins may spawn the next viewer instance (ex PartSeg allows for this) so you may have more than one viewer without script.

@Czaki I don't follow, can you elaborate on this use case?
Are these viewers without names? is the user expected to interact with them specifically?

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Dec 14, 2024

@dalthviz if you don't mind, lets make the pyqt6 fix bd8d5dc (#42) it's own PR, because this one may need some more discussion. it would make comparing main vs this easier!

@jni
Copy link
Member

jni commented Dec 14, 2024

Are these viewers without names?

The are not without names, but they are certainly in different Python frames: in one case, you launch the viewer in code, and in the other, you click a button within the first viewer and get a new viewer — the variable pointing to it is never going to be in the same namespace as the first viewer. As a totally inaccurate example, one would be "viewer" and the other would be somewhere like "viewer.window._qt_window._dock_widgets.getChildren()[0]". I think users would rather type current_viewer() than that. 😂

@Czaki
Copy link
Contributor

Czaki commented Dec 16, 2024

@Czaki I don't follow, can you elaborate on this use case?
Are these viewers without names? is the user expected to interact with them specifically?

In general, the use case is to compare the same datataset with different annotations.
When we have multicanvas, it may be less useful.

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

Successfully merging this pull request may close these issues.

Problem with viewer variable with multiple viewers and consolas
4 participants