feat: stack-viewer v2 using ndv as baseclass

[tlambert03]

this is an alternative to #293, that bases MDAViewer off of ndv which I broke into a new repo

[codecov]

Codecov Report

Attention: Patch coverage is 64.91228% with 40 lines in your changes missing coverage. Please review.

Project coverage is 89.94%. Comparing base (3df49c2) to head (9be8843).

Files	Patch %	Lines
...pymmcore_widgets/_stack_viewer_v2/_data_wrapper.py	45.45%	30 Missing ⚠️
...c/pymmcore_widgets/_stack_viewer_v2/_mda_viewer.py	81.13%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #299      +/-   ##
==========================================
- Coverage   90.24%   89.94%   -0.30%     
==========================================
  Files          76       79       +3     
  Lines        9585     9697     +112     
==========================================
+ Hits         8650     8722      +72     
- Misses        935      975      +40     

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

[wl-stepp]

Hi everyone,

we have been playing around with this a little the last two days. We have a couple of questions of how to use this in a full environment.
We like how the saving is handled if a sequence is started directly from the MDAWidget. But, I don't think there is way to get to the writers that are instantiated by that, right? My thinking was to get the writer to use it as a datastore as input for the MDAViewer.

  datastore = self.mmc.mda.handlers[0]
  self.viewer = MDAViewer(datastore)
  self.viewer.show()

So would it make sense for example to expose the handlers that have been connected to a run_mda?
https://github.com/pymmcore-plus/pymmcore-plus/blob/c13023ab801129797523c0dd13bfdb808e815efb/src/pymmcore_plus/mda/_runner.py#L239C1-L241C1
These are in a ContextManager however, so not sure what that would imply.

We have seen the way that this is implemented in
https://github.com/fdrgsp/micromanager-gui/blob/main/src/micromanager_gui/_core_link.py#L94
It just feels like that is redoing a lot of things that are in the MDAWidget in the first place already.

If we use the MDAViewer without input, we get a RAM based additional storage in the TensorStoreHandler, correct? Also, that display flickers with black frames if used on our setup. Might it be, that data is not written yet before the viewer tries to display it? Is there a better way to prevent this than patching frameReady?

Would appreciate some help.
Thanks!

[tlambert03]

We like how the saving is handled if a sequence is started directly from the MDAWidget. But, I don't think there is way to get to the writers that are instantiated by that, right? My thinking was to get the writer to use it as a datastore as input for the MDAViewer.

right, there isn't currently a way to do this, but with this NDViewer approach, I think it would be better if the MDAWidget created the handler and passed it to the runner, rather than passing in a string path. So that would be my preferred next step

There's of course many ways to go about this. I'm not crazy about self.mmc.mda.handlers[0]... i think i'd prefer a pattern like those in the examples here, where mda.run receives the handler, rather than creates the handler:

v = MDAViewer()
mmcore.run_mda(sequence, output=v.data)

or with an MDAWidget, the widget would create the handler and pass it to the MDAViewer. But, I kinda see all that as a question for a followup PR relating to the MDAWidget, and less about this viewer PR. From the prespective of the viewer, it can take or create a datastore (so it's flexible for whatever pattern is chosen later).

We have seen the way that this is implemented in
https://github.com/fdrgsp/micromanager-gui/blob/main/src/micromanager_gui/_core_link.py#L94
It just feels like that is redoing a lot of things that are in the MDAWidget in the first place already.

I agree, I wouldn't say that that's a recommended/final pattern. Federico is generally just playing with patterns there, and as you can see, has created his own MDAWidget subclass in that repo, just so he can get done what he wants to do. But definitely doesn't mean that that's where the final design is going

If we use the MDAViewer without input, we get a RAM based additional storage in the TensorStoreHandler, correct?

correct, but I imagine that ultimately the user will want more control over the handler, and that in most cases the MDAViewer will not be the one creating it.

Also, that display flickers with black frames if used on our setup. Might it be, that data is not written yet before the viewer tries to display it?

I'm aware of this in certain scenarios. is this with a two-channel image?

[wl-stepp]

right, there isn't currently a way to do this, but with this NDViewer approach, I think it would be better if the MDAWidget created the handler and passed it to the runner, rather than passing in a string path. So that would be my preferred next step

There's of course many ways to go about this. I'm not crazy about self.mmc.mda.handlers[0]... i think i'd prefer a pattern like those in the examples here, where mda.run receives the handler, rather than creates the handler:

Sounds good, will see where that takes us.

or with an MDAWidget, the widget would create the handler and pass it to the MDAViewer. But, I kinda see all that as a question for a followup PR relating to the MDAWidget, and less about this viewer PR. From the prespective of the viewer, it can take or create a datastore (so it's flexible for whatever pattern is chosen later).

👍 We'll test out some ways and see how it goes for us.

I agree, I wouldn't say that that's a recommended/final pattern. Federico is generally just playing with patterns there, and as you can see, has created his own MDAWidget subclass in that repo, just so he can get done what he wants to do. But definitely doesn't mean that that's where the final design is going

👍 Good work on the repo by the way @fdrgsp. Really helpful to see how you tie things together.

Also, that display flickers with black frames if used on our setup. Might it be, that data is not written yet before the viewer tries to display it?

I'm aware of this in certain scenarios. is this with a two-channel image?

No, single channel in this case. I will see if I can come up with a little more detail.

[wl-stepp]

So we have done some further work on this, but I can't get it to work the way I thought it would go.
I have been trying an approach as discussed:

handler = self._mmc.mda._handler_for_path(save_path)
self.viewer = MDAViewer(handler)
self._mmc.run_mda(sequence, output=handler)

But this gives an error when the ndv looks for the sizes

self = <pymmcore_widgets._stack_viewer_v2._data_wrapper.MM5DWriter object at 0x000002B9712EB050>

    def sizes(self) -> Sizes:
        """Return a mapping of {dimkey: size} for the data.
    
        The default implementation uses the shape attribute of the data, and
        tries to find dimension names in the `dims`, `names`, or `labels` attributes.
        (`dims` is used by xarray, `names` is used by torch, etc...). If no labels
        are found, the dimensions are just named by their integer index.
        """
        shape = getattr(self._data, "shape", None)
        if not isinstance(shape, Sequence) or not all(
            isinstance(x, int) for x in shape
        ):
>           raise NotImplementedError(f"Cannot determine sizes for {type(self._data)}")
E           NotImplementedError: Cannot determine sizes for <class 'pymmcore_plus.mda.handlers._ome_zarr_writer.OMEZarrWriter'>

I thought maybe we would have to pre-initialize the sequence, but that also doesn't help.

handler = self._mmc.mda._handler_for_path(save_path)
sequence = self.value()
handler.sequenceStarted(sequence)
self.viewer = MDAViewer(handler)
self.execute_mda(handler)

I can work around this in ndv just to try by

    def sizes(self) -> Sizes:
        try:
            return self._data.current_sequence.sizes
        except Exception as e:
            print(e)
            shape = getattr(self._data, "shape", None)
            if not isinstance(shape, Sequence) or not all(
                isinstance(x, int) for x in shape
            ):
                raise NotImplementedError(f"Cannot determine sizes for {type(self._data)}")
            dims = range(len(shape))
            return {dim: int(size) for dim, size in zip(dims, shape)}

This runs sometimes despite the KeyError but most of the times crashes on

  File "C:\Control_2\pymmcore-widgets\src\pymmcore_widgets\_stack_viewer_v2\_data_wrapper.py", line 86, in isel
    data = self._data.position_arrays[self._data.get_position_key(p_index)]
           ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'p0'

useq-schema 0.4.8.dev41+g3eea067
pymmcore-widgets 0.1.dev248+g53e9ddf
pymmcore-plus 0.11.0
ndv 0.1.dev53+gb79241c.d20240719

Let me know if I'm doing something wrong here

[tlambert03]

yeah, sorry @wl-stepp, this is definitely still in a not-quite-ready state. and dealing with indeterminate sizes is one of the things that I think I previously had working, and then broke with some changes on the ndv side. so you're correct, it needs some updates here. I apologize for leaving this in an intermediate state, I know it's high value... I had hoped to solidify some of the patterns in ndv before committing to a pattern here, but that ended up going down an (as of yet unresolved) rabbit hole. I either need to just commit to the current ndv pattern and make it work here, or find the time to make the changes I wanted over there (related to pyapp-kit/ndv#33)

[wl-stepp]

Sure, no problem. Just to document what I run into first.