feat: ArrayDisplayModel (for later model/view refactor)

[tlambert03]

this is the model-only partly from #33, cleaned up for easier review. This isn't yet connected to a view, but can be reviewed for soundness of the model itself, interactive validation (i.e. instantiate the object and see if you can break it with invalid inputs), and signal emission (they should be appropriate for later view connection).

see the display_model.py example for a starting point

cc @gselzer if you want to play with it and provide feedback (including requests for more docstrings & comments for any parts that seem confusing or magical), or try to connect it to a view

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 3.11987% with 590 lines in your changes missing coverage. Please review.

Project coverage is 56.00%. Comparing base (10abe13) to head (1b07aad).

Files with missing lines	Patch %	Lines
src/ndv/controller.py	0.00%	150 Missing :warning:
src/ndv/v2view_qt.py	0.00%	127 Missing :warning:
src/ndv/models/_mapping.py	0.00%	106 Missing :warning:
src/ndv/v2view_jupyter.py	0.00%	86 Missing :warning:
src/ndv/models/_array_display_model.py	0.00%	49 Missing :warning:
src/ndv/models/_reducer.py	0.00%	30 Missing :warning:
src/ndv/models/_lut_model.py	0.00%	16 Missing :warning:
src/ndv/viewer/_data_wrapper.py	50.00%	14 Missing :warning:
src/ndv/models/_base_model.py	0.00%	6 Missing :warning:
src/ndv/models/__init__.py	0.00%	3 Missing :warning:
... and 1 more

:exclamation: There is a different number of reports uploaded between BASE (10abe13) and HEAD (1b07aad). Click for more details.

HEAD has 4 uploads less than BASE

| Flag | BASE (10abe13) | HEAD (1b07aad) | |------|------|------| ||5|1|

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #51 +/- ## =========================================== - Coverage 78.93% 56.00% -22.93% =========================================== Files 13 22 +9 Lines 1856 2430 +574 =========================================== - Hits 1465 1361 -104 - Misses 391 1069 +678 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[tlambert03]

am wondering about your plans for modeling the other data involved, namely the array data itself

just pushed some of my recent playground changes (in viewer_v2.py) where you can see how I was envisioning this. Basically, the array display model is intentionally expressed independent of the data itself. But of course you will need to pair it with a DataWrapper in order to get much use. I'm currently playing with that in the Viewer object itself, but if there's sufficient logic just between that pair of objects, it could be extracted to an intermediate object.

whether this model could handle multiple datasets. I think that, written right now, it could, but is that a "feature" we should preserve later, or just an aspect of the current state it maintains?

yes that is definitely a goal here (and is part of why i exclude the dataset from the display model). I don't think it's a big can of worms really... it just requires a more explicit model of a "world object" (i.e. a thing that has a data-to-world transform) that tells us how to place the object in the scene graph. Essentially, right now you can think of it as everything having an (implicit) data-to-world transform of "identity".

basically, i think everything's still on a good path here for these things

I'd love to try hooking this up to a widget and see how far I/we can get.

see x.py for now in this PR. Note, at this point it's just in "hook all the things up mode" ... not clean and not refactored to be clear

[tlambert03]

m.current_index["5"] = slice(1, "fooo")

the only issue there is the "fooo" in the slice right? I can update the Slice type validator

[gselzer]

just pushed some of my recent playground changes (in viewer_v2.py) where you can see how I was envisioning this. Basically, the array display model is intentionally expressed independent of the data itself. But of course you will need to pair it with a DataWrapper in order to get much use. I'm currently playing with that in the Viewer object itself, but if there's sufficient logic just between that pair of objects, it could be extracted to an intermediate object.

Oh, great!

Since you're already working on the "view" part...how much are you planning to separate the "view" part (Qt/Jupyter, vispy/pygfx) from the "controller" part (signal connections, maybe some top-level user API)? Might be helpful in resolving #48

yes that is definitely a goal here (and is part of why i exclude the dataset from the display model). I don't think it's a big can of worms really... it just requires a more explicit model of a "world object" (i.e. a thing that has a data-to-world transform) that tells us how to place the object in the scene graph. Essentially, right now you can think of it as everything having an (implicit) data-to-world transform of "identity".

basically, i think everything's still on a good path here for these things

Glad to hear it! Since we're talking about transforms, do you think it's worth allowing current_index values be numbers (i.e. int or float)?

see x.py for now in this PR. Note, at this point it's just in "hook all the things up mode" ... not clean and not refactored to be clear

❤️

the only issue there is the "fooo" in the slice right? I can update the Slice type validator

Yeah, think so!

[tlambert03]

how much are you planning to separate the "view" part (Qt/Jupyter, vispy/pygfx) from the "controller" part (signal connections, maybe some top-level user API)? Might be helpful in resolving

was definitely also in scope for this :) i just split some stuff up and now there's an mvc.py pattern. Still a mess, but you can run it. Gives a decent example of how we can split all front-end concerns

[gselzer]

was definitely also in scope for this :) i just split some stuff up and now there's an mvc.py pattern. Still a mess, but you can run it. Gives a decent example of how we can split all front-end concerns

Gorgeous 😍 this was exactly what I was envisioning, and I was hacking out something like this before I saw your WIP commit yesterday.

[tlambert03]

great! makes me happy to hear it's along the lines of what you were picturing

[tlambert03]

something I'm still thinking about a lot (as you were above) is the relationship between the data wrapper and the display model.

I do definitely want it to be possible to have the same display model associated with two different data wrappers (this is essentially like "synced windows" in Fiji, where you link the current index and luts of two different images). And I also want it to be possible to associate two different display models with the same data wrapper (this would make orthoviewers trivial). So I don't think I want to simply add a data field to the ArrayDisplayModel. But i do find it a little clunky to pass around both the "model" and the "data" (since they are rather linked with each other). So perhaps an intermediate object:

class DisplayedData:
    data: DataWrapper | None
    view_model: ArrayDisplayModel

That object would have logic like the _canonicalize_axis_key method. That method is essentially there because we do want someone to be able to express axis keys in the display in as flexible of a way as possible (including negative indexing, named axes, etc... visible_axes = (-2, 1) or visible_axes = {'y', 'x'}). But at the end of the day, those keys must be resolved to a specific axis index, and for that we need the data. So that's a good place for it i think

[tlambert03]

Since we're talking about transforms, do you think it's worth allowing current_index values be numbers (i.e. int or float)?

this concept is important if we want to select a display in world space (instead of data space), but that sort of thing was such a hairy ball of wax in napari, that I want to leave all that kind of multi-world-object negotiation for another PR

[gselzer]

That object would have logic like the _canonicalize_axis_key method. That method is essentially there because we do want someone to be able to express axis keys in the display in as flexible of a way as possible (including negative indexing, named axes, etc... visible_axes = (-2, 1) or visible_axes = {'y', 'x'}). But at the end of the day, those keys must be resolved to a specific axis index, and for that we need the data. So that's a good place for it i think

It's possible that I just haven't dug as deep in as you have yet, but why can that not be resolved in the view, or in the controller? I do like the idea of thinking about the "Viewer" having three things, which it is responsible for interconnecting:

1. data: the information being displayed
2. model: how to display data (this definition, which is basically copied from _array_display_model, could use a little wordsmithing to sufficiently distinguish from the view)
3. view: the mechanism of display

[gselzer]

I've also been hacking more on #49 in the meantime, now off of your prototype. Do you think that it makes sense to build up StatsModel and StatsView objects in tandem with the objects you're building here?

To be clear, I'm not suggesting we do these things in this PR - if this is something we want, I'll build it separately, working for the current NDViewer design, and we can work it into your final design later

[tlambert03]

Do you think that it makes sense to build up StatsModel and StatsView objects in tandem with the objects you're building here?

I do actually think that model view could work there. That's a better way to phrase the vague things I was saying about some "stats service" (i.e. model), the thing that can calculate min max mean histogram, etc... once, and be shared by various things, like the auto-scaler, the histogram, etc...

[gselzer]

Yup, and I also like the idea of a view protocol because it could make it easier to define multiple histogram widgets as I describe in #49 - a simple, small one that could replace dimension sliders, and a more complicated widget like you had in napari/napari#675 that could be brought up with double click...

[tlambert03]

proof of principle working with Jupyter :)

https://github.com/user-attachments/assets/041de589-ae2e-400b-bb92-c34776286224

[tlambert03]

to prevent confusion, I'm going to close this PR. As I told @gselzer, this branch is now present upstream at https://github.com/pyapp-kit/ndv/tree/v2-mvc ... and to facilitate multiple people (me and gabe) working on different aspects of it at the same time, all PRs working on the model view refactor will be made to that v2-mvc branch instead of to main. Then, we will eventually merge that branch into main in one PR