napari: proposal: reducing complexity of models

🚀 Feature

Reduce complexity of current model code by factoring out user-facing API as a view, business logic as a controller, and data structures as a model.

Motivation

Model code is getting excessively complex. Files are typically over 1000 lines long and difficult to parse/keep track of what’s going on. Currently, our models are dealing with data management, presentation logic (i.e. Python magic & data conversion to make it better for users), and business logic.

Right now I feel like we have to make tradeoffs between nice UX and maintainability for the API and separating these components makes it easier to maintain without sacrificing our nice API. Take for example how we handle private variables: either they go without any documentation and can be confusing to the developer or they wind up awkwardly in the public-facing documentation where it doesn’t really belong and confuses the user.

Furthermore, I think that the current user-facing API functions more like a view since we do a lot of input/output conversions and use custom Python syntax for a nicer user experience. Perhaps the best example of this is the LayerList.

Pitch

I want to split out the current model code into model, view, and controller classes.

Model

The model would be a simple data class that notifies when its fields are changed.

from dataclasses import dataclass

@dataclass
class PointsLayerModel(BaseLayerModel):
    data: Array = notifier()
    mode: Mode = notifier()
    ...

This makes it easy to keep track of all relevant information pertaining to the state. You can even easily convert this state into a dictionary for saving and sharing with dataclasses.asdict. Keeping this class simple may also make it easier to handle settings and create autogenerated code from the state.

View

The view would be a simple shell that mimics what you can do with the GUI. It emits events when a user interacts with it and may clean the input before handing it off to the controller.

class Points(Layer):
    @property
    def mode(self):
        return str(self._model.mode)

    @mode.setter
    def mode(self, mode):
        self.events.mode(value=Mode(mode))

Controller

The controller handles the business logic, connects the model to the view, and modifies the model. In a lot of cases it will be a simple event handler that prevents setters from being called multiple times (this can be autogenerated).

class PointsLayerController(BaseLayerController):
    def on_mode_change(self, event):
         with event.source.events.mode.blocker():
             self.model.mode = event.value

    def on_data_change(self, event):
         data = event.value
         data_view = ...
         # calculation of data view goes here
         self.model.data_view = data_view

Alternatives

We could factor out the view without separating the model and the controller and it wouldn’t be too big of an issue, but I believe that it would make it easiest to maintain in the future.

Additional context

Flow of current structure (diagram by @shanaxel42):

Flow of structure in #1040 (diagram by @shanaxel42):

Flow of proposed structure: Screen Shot 2020-06-01 at 9 05 53 PM

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 16 (11 by maintainers)

Most upvoted comments

Thanks to everyone for weighing in! It seems that there is pretty strong consensus towards keeping the public API a model, so I’ll ditch API View idea, but I’m glad that people are considering the use of a controller (no surprise from @shanaxel42 tho since she was the one who first proposed it 😛). Maybe we can use this issue to work out what belongs in the controllers vs everything else.

@0x00b1 I’d love to work on this with you! I don’t have any experience in architecture/other stuff so I would really appreciate your knowledge on what to do 😃

right now with our setters events don’t get emitted when we change them in place (i.e. layer.contrast_limits[1] = 10 won’t trigger an update, which is confusing if we imagine we have a python api that enables this

@sofroniewn you’ll need to create custom classes that mimic python data structures like what we did with LayerList. However, I’m not sure how much @jni would be a fan of this 😛

kne42 on Jun 4, 2020

Hello! First off I know I’ve been super out of the loop on things so my opinions here may not be up to data with the current discussions. But wanted to give some input. Thanks @kne42 for writing this up, and especially for making the diagrams.

So what I like about this proposal is the addition of the “controller” as the place where you cleanup data before updating the model or add other logic. It seems like the main issue around the model files getting bloated could be solved by just adding this layer.

I see the appeal of wanting to make the separate view file for interacting with the model but I’m with @jni that “The model should be the API”. At lest for the developer use case.

I would also strongly discourage moving in any direction that continues to use the blocker pattern, it just creates a high likelihood of infinite loops or repeat calls and was one of the main things I was trying to eliminate in my original proposal.

Napari is unique in that it supports interfacing with the data from both the GUI and the data directly. And now a third use case - plugins. If you wanted to incorporate the addition of controllers my suggestion would be to have any use case besides direct data manipulation go through the controller. So what this looks like for each use case:

Developer Scenario a user wants to interact with the data directly. They should be able to directly update the models which emit an event that the event handler picks up to update the views.
GUI user scenario a user interacts with the data though the GIU, an event is emitted, the event handler picks it up and calls the controller’s update method which then updates the data model
Plugin Scenario A plugin interacts with the models by calling update methods defined in the controller. This also has the advantage of more control over what a plugin can/cannot do with models.

Anyway my 2 cents, thanks @kne42 for all your hard work on this!

shanaxel42 on Jun 3, 2020

This is very interesting, and I’ll need to spend more time thinking about it before weighing in, but one think I want to add now, is that at this point any major redesign should really put ideas for our plugin interface and headless mode at the front and center too.

In my opinion, the plug-in interface shouldn’t make any assumptions about the user interface (or even the existence of a user interface). As a teenager in the late-90s, I have endearing, nostalgic memories of insane VST plug-in interfaces, but that’s not something I want to revisit!

0x00b1 on Jun 3, 2020

Probably? I haven’t been active on this repo in a while, can you catch me up to speed on what we’re doing with those?

kne42 on Mar 29, 2022

@kne42 I love the diagram. My suggestion:

I think this has the benefit of making it orthogonal to plug-in discussions.

0x00b1 on Jun 3, 2020

I also quite strongly dislike having the API view be different from the model. The model should be the API, in my opinion. The Pythonic way is to have direct access to the data. In other words, from napari.layers import Image should be the model and the preferred way to interact with said model. It’s unclear to me whether in your diagram, that import would give me the API View, the Model, or the API view but also instantiate the Controller and the Model.

💯

I agree, in an ideal world. But I think that we’re getting to the point where the code is so messy that we can no longer stick to our ideals. It is much more pragmatic to improve maintainability, especially if nothing changes on the user end. Furthermore, separating out these aspects of the program will make it much easier to fool-proof the API if we wish, in the event that we no longer want to take a “garbage in garbage out” approach.

Napari hasn’t reached a 1.0 release, there’s no Napari publication, and (as far as I know) there’re few Napari users. From a maintenance perspective, publicizing classes like napari.layers.Image (and encouraging their use) will help ensure Napari’s fundamental data structures remain (or, depending on your perspective, become) ergonomic, tested, and documented.

0x00b1 on Jun 3, 2020