Capture caller frame of napari.run() when starting the console.

[Carreau]

Should fix napari/napari#4098
Replaces napari/napari#4140, and see discussion there as well
Needs napari/napari#4212

[alisterburt]

napari/napari#4212 has merged now, should we merge this too @Carreau? Changes look good to me, although there are a few strange extra blank lines :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.38%. Comparing base (29b68a3) to head (94e8cbc).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
+ Coverage   87.60%   90.38%   +2.78%     
==========================================
  Files           4        4              
  Lines         121      156      +35     
==========================================
+ Hits          106      141      +35     
  Misses         15       15              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[psobolewskiPhD]

I installed this PR branch and napari main. No more errors, but I must be missing something, because, I made a script:

import napari
a = 3
b = 4
viewer = napari.Viewer()
c = 5
napari.run()

I was expecting c to be available in the napari console, but it's not.
🤔

[Carreau]

as we inspect frames my guess is we see the frame at the point the viewer was created so before C ?

…

On Wed, Dec 21, 2022 at 21:48 Peter Sobolewski ***@***.***> wrote: I installed this PR branch and napari main. No more errors, but I must be missing something, because, I made a script: import napari a = 3 b = 4 viewer = napari.Viewer() c = 5 napari.run() I was expecting c to be available in the napari console, but it's not. 🤔 — Reply to this email directly, view it on GitHub <#18 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACR5TZJI3BOYHNNT6OGJLLWONULLANCNFSM5QDP6SXA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[psobolewskiPhD]

Ah, I assumed it would be stuff after the viewer.
But checking again, a and b arn't available either.
☹️

[Carreau]

Yeah, I forgot to push...

[Carreau]

Or technically I pushed, but did not commit, so it pushed nothing.

[brisvag]

I was expecting c to be available in the napari console, but it's not.

that would be great too, but probably more complicated cause it requires back and forth communication?

[psobolewskiPhD]

Or technically I pushed, but did not commit, so it pushed nothing.

Ah! I'm so pleased it wasn't me screwing something up again! 😊
Anyhow, refreshing the branch and it works perfectly:

a
Out[1]: 3

b
Out[2]: 4

c
Out[3]: 5

[Czaki]

I have mixed feelings. It is really nice in the context of scripts or notebooks.

But for example, PartSeg allow opening napari viewer from the view menu. And with this PR the context of this method is pushed:

https://github.com/4DNucleome/PartSeg/blob/3f8f3bc56e1fa055f65d428b1d9e6d776af8ae37/package/PartSeg/common_gui/main_window.py#L230-L235

I do not feel that it is proper behavior.

[Czaki]

What with modules named napari-something?

[jni]

should we change these to

Suggested change

	for pref in ["napari", "in_n_out"]:
	for pref in ["napari.", "in_n_out."]:

then?

[psobolewskiPhD]

@Czaki can you elaborate what is the issue with your PartSeg use case?
Does it maybe make sense to have the console disabled in that case?

[Czaki]

@Czaki can you elaborate what is the issue with your PartSeg use case?

exporting internal application variables to the console. (It could be easily workaround - for a person like me who understands napari internals)

Does it maybe make sense to have the console disabled in that case?

I use it regularly in the context of debugging and development. And some person uses it to perform actions not possible by GUI (ex layer scale).

[psobolewskiPhD]

Gotcha. Makes sense. Maybe we need an argument to not do this when launching napari?
Even from a script someone may not want the variables passed?

[jni]

Can we try to push this over the line? Todos:

• check my suggestion on the prefixes
• figure out solution for libraries that use napari as an internal tool; two half-baked suggestions:
  • add a "max_depth=1" kwarg for the frame, so that this only works for scripts (depth=0) and a single function (depth=1), but is easily configurable if we find that doesn't cover everything. (Not sure whether this works in IPython?)
  • add a "blacklist" of modules where we don't do this, including partseg

For point 2, I wouldn't mind leaving it to a later PR, as this is already extremely useful.

[Czaki]

I like option 1.

I'm not the favorite of option 2. It should be rather information in the documentation that will inform about this and suggest ways to workaround it. (maybe such variable _not_incude_in_napari_console=True).

[jni]

If you yourself are happy with your proposal can you dismiss your review and we can merge? 😃

implemented

[Czaki]

@jni I have added tests and updated the readme. Could you check it?

[jni]

Well the tests seem unhappy... 😅

[Czaki]

Well the tests seem unhappy... 😅

I forget that there is a global state in the console, so names in tests cannot collide.

[jni]

I updated the README message to include a usage example.

[Czaki]

This is not true. I implement it as global module-level variable, not an environment variable. Should I change it?

[jni]

Oh! Sorry. I did read that and thought you misspoke. 😅 I think I prefer this way because it's easier to describe and quite a common way to change functionality. Otherwise you have to make sure that the global is accessible in the correct frame, which is not something people are used to thinking about, I think? 🤷

[Czaki]

yes, but environment variable is global per process, and module level variable is in module.

If someone would like first spawn PartSeg and then napari, If I use an environment variable, then it will prevent variable propagation in follow-up napari start.

If using module-level variable it impacts only possible console spawn when open console from napari embedded here. But will not impact napari spawned from the top-level script.

[brisvag]

Suggested change

	Predicates that return Wether we are in napari by looking
	at:
	1) the frames modules names:
	2) the min_depth
	Determines whether we are in napari by looking at:
	1) the frames modules names:
	2) the min_depth

[jni]

The only dodgy one is action_manager at the bottom, dunno what's going on there...

(from me at #18 (comment))

@Carreau it looks like that was added by you in napari/napari#2677 without any discussion. I see now in the PR description your comment, "I also inject action_manager in the qtconsole context, so one does not have to import it everytime", but in general I think the action manager is not really a user-facing object, even though it is convenient for people developing specific things. Was that for debugging and can it be removed? (This would be for 0.5.0 or later in any case...)

[Carreau]

I see now in the PR description your comment, "I also inject action_manager in the qtconsole context, so one does not have to import it everytime",

I belive this was done before we had the shortcut editor so we needed a way to edit shortcuts is a relatively friendly way.

[jni]

Well, I suggest that we merge this and fix the action_manager injection in another PR.

[jni]

(It would be cool to simul-release this improvement with 0.4.19!)

[jni]

Shit, I should have tested this again before merging 😅

Right now when I run python examples/add_labels.py, I see the viewer, np, napari, and some IPython stuff:

In [1]: locals()
Out[1]: 
{'__name__': '__main__',
 '__doc__': 'Automatically created module for IPython interactive environment',
 '__package__': None,
 '__loader__': None,
 '__spec__': None,
 '__builtin__': <module 'builtins' (built-in)>,
 '__builtins__': <module 'builtins' (built-in)>,
 '_ih': ['', 'locals()'],
 '_oh': {},
 '_dh': [PosixPath('/Users/jni/projects/napari')],
 'In': ['', 'locals()'],
 'Out': {},
 'get_ipython': <bound method InteractiveShell.get_ipython of <ipykernel.inprocess.ipkernel.InProcessInteractiveShell object at 0x301fd1310>>,
 'exit': <IPython.core.autocall.ZMQExitAutocall at 0x3030068d0>,
 'quit': <IPython.core.autocall.ZMQExitAutocall at 0x3030068d0>,
 'open': <function io.open(file, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)>,
 '_': '',
 '__': '',
 '___': '',
 'viewer': Viewer(camera=Camera(center=(0.0, np.float64(101.0), np.float64(141.5)), zoom=np.float64(1.7528169014084505), angles=(0.0, 0.0, 90.0), perspective=0.0, mouse_pan=True, mouse_zoom=True), cursor=Cursor(position=(1.0, 1.0), scaled=True, style=<CursorStyle.STANDARD: 'standard'>, size=np.float64(10.0)), dims=Dims(ndim=2, ndisplay=2, order=(0, 1), axis_labels=('0', '1'), rollable=(True, True), range=(RangeTuple(start=np.float64(0.0), stop=np.float64(202.0), step=np.float64(1.0)), RangeTuple(start=np.float64(0.0), stop=np.float64(283.0), step=np.float64(1.0))), margin_left=(0.0, 0.0), margin_right=(0.0, 0.0), point=(np.float64(101.0), np.float64(141.0)), last_used=0), grid=GridCanvas(stride=1, shape=(-1, -1), enabled=False), layers=[<Image layer 'coins' at 0x1750fa190>, <Labels layer 'segmentation' at 0x1750f9a50>], help='use <7> for transform, use <1> for activate the label eraser, use <2> for activate the paint brush, use <3> for activate the polygon tool, use <4> for activate the fill bucket, use <5> for pick mode', status='Ready', tooltip=Tooltip(visible=False, text=''), theme='dark', title='napari', mouse_over_canvas=False, mouse_move_callbacks=[], mouse_drag_callbacks=[], mouse_double_click_callbacks=[], mouse_wheel_callbacks=[<function dims_scroll at 0x1647e4040>], _persisted_mouse_event={}, _mouse_drag_gen={}, _mouse_wheel_gen={}, _keymap={}),
 'napari': <module 'napari' from '/Users/jni/projects/napari/napari/__init__.py'>,
 'action_manager': <napari.utils.action_manager.ActionManager at 0x15e051910>,
 'np': <module 'numpy' from '/Users/jni/micromamba/envs/all/lib/python3.11/site-packages/numpy/__init__.py'>,
 '_i': '',
 '_ii': '',
 '_iii': '',
 '_i1': 'locals()'}

I wonder if this commit broke things?

5339095

[jni]

Indeed changing this:

if c.frame.f_globals.get("__name__", "") != "__main__" and "NAPARI_EMBED" not in c.frame.f_globals:

to

if "NAPARI_EMBED" not in c.frame.f_globals:

fixes it again. I'll make a PR, since I don't think the first part is necessary (?) @Czaki