pyapp-kit / magicgui

build GUIs from type annotations
https://pyapp-kit.github.io/magicgui/
MIT License
365 stars 49 forks source link

decorating a function that uses `napari.viewer.add_points` with magicgui generates a Shader compilation error #147

Closed VolkerH closed 3 years ago

VolkerH commented 3 years ago

Describe the bug

I have some sample code here

https://gist.github.com/VolkerH/d732ab350113deb347fe4beafd778993

This is not a minimally reproducible example yet.

If I do not decorate mosaic_well(...) with @magicgui everything runs fine. A mosaic is loaded, the tiles are displayed and the anchor points for each tile are plotted. If I do decorate with magicgui the code crashes at this line when adding the points: ``pts = viewer.add_points(coords_numpy / subsample, size=10, face_color="red") I will include the traceback further down.

Removing the viewer.add_points line prevents the crashing and I can view the mosaic. I can't make sense of it.

To Reproduce See above.

Expected behavior Decorating the function shoud not cause shader compilation errors.

Environment (please complete the following information):

OpenGL:

Screens:

Plugins:

Traceback that I observe with decorator:

$ python napari_nikon_mosaic.py 
<class 'magicgui.widgets._function_gui.FunctionGui'>
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0000.tif [0. 0.]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0001.tif [-24967.65625  11804.53125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0002.tif [-26821.40625  11815.     ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0003.tif [-28675.     11825.625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0004.tif [-30528.59375  11836.09375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0005.tif [-32382.1875   11846.71875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0006.tif [-34235.78125  11857.1875 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0007.tif [-36089.375    11867.65625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0008.tif [-37943.125    11878.28125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0009.tif [-37953.75     10010.15625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0010.tif [-36100.        9999.53125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0011.tif [-34246.40625   9989.0625 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0012.tif [-32392.8125   9978.4375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0013.tif [-30539.21875   9967.96875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0014.tif [-28685.625   9957.5  ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0015.tif [-26832.03125   9946.875  ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0016.tif [-24978.28125   9936.40625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0017.tif [-23124.6875    9925.78125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0018.tif [-23135.3125    8057.65625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0019.tif [-24988.90625   8068.28125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0020.tif [-26842.5    8078.75]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0021.tif [-28696.25    8089.375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0022.tif [-30549.84375   8099.84375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0023.tif [-32403.4375   8110.3125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0024.tif [-34257.03125   8120.9375 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0025.tif [-36110.625     8131.40625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0026.tif [-37964.21875   8142.03125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0027.tif [-37974.84375   6273.90625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0028.tif [-36121.25      6263.28125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0029.tif [-34267.65625   6252.8125 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0030.tif [-32414.0625   6242.1875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0031.tif [-30560.46875   6231.71875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0032.tif [-28706.875   6221.25 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0033.tif [-26853.125   6210.625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0034.tif [-24999.53125   6200.15625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0035.tif [-23145.9375    6189.53125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0036.tif [-23156.5625    4321.40625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0037.tif [-25010.15625   4332.03125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0038.tif [-26863.75   4342.5 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0039.tif [-28717.34375   4353.125  ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0040.tif [-30571.09375   4363.59375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0041.tif [-32424.6875   4374.0625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0042.tif [-34278.28125   4384.6875 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0043.tif [-36131.875     4395.15625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0044.tif [-37985.46875   4405.78125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0045.tif [-37996.09375   2537.65625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0046.tif [-36142.5       2527.03125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0047.tif [-34288.90625   2516.5625 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0048.tif [-32435.3125   2505.9375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0049.tif [-30581.71875   2495.46875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0050.tif [-28727.96875   2485.     ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0051.tif [-26874.375   2474.375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0052.tif [-25020.78125   2463.90625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0053.tif [-23167.1875    2453.28125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0054.tif [-23177.8125     585.15625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0055.tif [-25031.40625    595.78125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0056.tif [-26885.      606.25]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0057.tif [-28738.59375    616.875  ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0058.tif [-30592.1875     627.34375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0059.tif [-32445.9375    637.8125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0060.tif [-34299.53125    648.4375 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0061.tif [-36153.125      658.90625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0062.tif [-38006.71875    669.53125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0063.tif [-38017.34375  -1198.75   ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0064.tif [-36163.75     -1209.21875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0065.tif [-34310.15625  -1219.6875 ]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0066.tif [-32456.5625  -1230.3125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0067.tif [-30602.8125   -1240.78125]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0068.tif [-28749.21875  -1251.40625]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0069.tif [-26895.625  -1261.875]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0070.tif [-25042.03125  -1272.34375]
/home/hilsenst/Desktop/W2/Well1_Seq0001_1_0071.tif [-23188.4375   -1282.96875]
subsample type <class 'int'>
WARNING: Error drawing visual <Text at 0x7fb08fa43210>
ERROR:root:Unhandled exception:
Traceback (most recent call last):
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/app/backends/_qt.py", line 825, in paintGL
    self._vispy_canvas.events.draw(region=None)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/util/event.py", line 455, in __call__
    self._invoke_callback(cb, event)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/util/event.py", line 475, in _invoke_callback
    self, cb_event=(cb, event))
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/util/event.py", line 471, in _invoke_callback
    cb(event)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/scene/canvas.py", line 217, in on_draw
    self._draw_scene()
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/scene/canvas.py", line 266, in _draw_scene
    self.draw_visual(self.scene)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/scene/canvas.py", line 304, in draw_visual
    node.draw()
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/scene/visuals.py", line 99, in draw
    self._visual_superclass.draw(self)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/visuals/visual.py", line 595, in draw
    v.draw()
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/scene/visuals.py", line 99, in draw
    self._visual_superclass.draw(self)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/visuals/visual.py", line 443, in draw
    self._vshare.index_buffer)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/visuals/shaders/program.py", line 101, in draw
    Program.draw(self, *args, **kwargs)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/program.py", line 533, in draw
    canvas.context.flush_commands()
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/context.py", line 176, in flush_commands
    self.glir.flush(self.shared.parser)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/glir.py", line 572, in flush
    self._shared.flush(parser)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/glir.py", line 494, in flush
    parser.parse(self._filter(self.clear(), parser))
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/glir.py", line 819, in parse
    self._parse(command)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/glir.py", line 787, in _parse
    ob.set_data(*args)
  File "/home/hilsenst/miniconda3/envs/napari/lib/python3.7/site-packages/vispy/gloo/glir.py", line 930, in set_data
    (self._target, errormsg))
RuntimeError: Shader compilation error in 35633:
tlambert03 commented 3 years ago

will take a look... can you tell me what coordinate_utils is?

VolkerH commented 3 years ago

I added coordinate_utils.py to the gist above. Basically just a few functions that read stage positions from a .txt based metadata file and return them as a pd.DataFrame.

I will also try and gut out the code to create a minimal reproducible example in the next half hour.

tlambert03 commented 3 years ago

Oh, and total side note:

You can put viewer: "napari.viewer.Viewer" in the decorated function signature to get the current napari viewer (rather than creating it in the function)

VolkerH commented 3 years ago

Oh, and total side note:

You can put viewer: "napari.viewer.Viewer" in the decorated function signature to get the current napari viewer (rather than creating it in the function)

I was just wondering about where to create the viewer. What is the significance of enclosing the type as a string there? This is some sort of Python construct that I have not previously encountered.

This error is rather weird, I haven't succeeded to get a minimal example there.

tlambert03 commented 3 years ago

What is the significance of enclosing the type as a string there? This is some sort of Python construct that I have not previously encountered.

That's a lovely typing construct called a forward reference. It's optional (in that you can also use the non-string form) ... but it lets you annotate something as being of a certain type without actually having to import that thing. So these two have more or less the same result from a typing perspective:

from napari import Viewer

@magicgui
def func(viewer: Viewer): ...
from typing import TYPE_CHECKING

# the TYPE_CHECKING clause is also optional, but if you include it
# IDEs will be able to resolve your forward references
# and you can continue to not directly depend on them
if TYPE_CHECKING:
    import napari

@magicgui
def func(viewer: 'napari.viewer.Viewer'): ...
VolkerH commented 3 years ago

Thanks! I like type annotations but I never dug that deep. They almost seem to be a whole language on top of Python in itself.

Regarding my original problem with this issue. I made some changes, including passing the existing viewer as suggested as well as docking the widget in napari:

https://gist.github.com/VolkerH/68235e3ed954bfd7c612ed29fb779546

This version works as intended and does not crash. I still don't know why the other one did. Maybe the presence of the separate floating Qt dialog caused some issues.

tlambert03 commented 3 years ago

This version works as intended and does not crash.

huh, well I'd still like to figure out why the other one did too. what changed here outside of passing the viewer? If I run the original updated gist, should it crash for me?

VolkerH commented 3 years ago

Well, to reproduce the original you probably need the dataset that I used. Let me know if you want it and I will send you a dropbox link (can't share it here).
I tried a few things to create a small reproducible example, such as replacing the image loading with random array generation but I was not able to make it crash in the same manner.

tlambert03 commented 3 years ago

actually, @VolkerH ... looking at this again, there's another thing I should have mentioned about magicgui and napari... If you want the output of a magicgui function to be added to the napari viewer, the "most tested" way is to use a special return annotation in your function. this might let you avoid the viewer object and your showfiles function altogether:

# untested ... but here's the basic idea

@magicgui(call_button="Show Mosaic", auto_call=False)
def mosaic_well(
    settingsfile: Path,
    viewer: "napari.viewer.Viewer",
    subsamplefactor: int = 2,
    channel: int = 0,
    directionx: int = 1,
    directiony: int = 1,
    crop: int = 12,
    do_transpose: bool = True,
) -> "napari.types.LayerData":

    ...

    # where layer_data is a list of tuples,
    # and each tuple has up to three members: (data, {metadata}, 'layer_type')
    layer_data = []
    for name, coord in zip(files, coords_numpy):
        translate = ([coord[0] / subsample, coord[1] / subsample],)
        layer_data.append((getimage(name), {"translate": translate}))
    layer_data.append(
        (coords_numpy / subsample, {"size": 30, "face_color": "red"}, "points")
    )
    return layer_data

The general idea here is that programs like napari can register both visual representations and behavior for a specific type. In this case, napari has registered that "if a function is annotated with a return type of napari.types.LayerData, then add each of the layerdata tuples to whatever viewer that magicgui widget has been added to ...

there are more of these "special" types (like napari.types.ImageData... which can just return a np.array) needs better docs, sorry

VolkerH commented 3 years ago

Thanks Talley, ... even more magic ..., will give that a go.

tlambert03 commented 3 years ago

@VolkerH, let me know if you'd like to keep this open. Sounds like you have a fix by not directly creating and working with the viewer inside the decorated function right?

VolkerH commented 3 years ago

Sorry Talley, I lost track. I'll close it for now as I cannot reproduce with a minimal example. The issue with hidden magic is sometimes that it is a bit harder to debug for non-magicians. (not a criticism of magicgui, I love it, more an apology for not providing more detailed info on this one).

tlambert03 commented 3 years ago

no apology needed. the napari-specific magic is woefully lacking in documentation. "magic" that performs a series of commonly performed tasks without much effort is awesome. "magic" that does invisible shit that you have no idea about is useless! :joy: I'll write something up soon.

feel free to reopen this if you find a MRE