AllenCell / AllenCell/allencell-segmenter-ml

Refactor state management for main tabs

Ouverte
#404 0 commentaires 0 réactions 3 personnes assignées Réclamée par @yrkim98 Voir sur GitHub
refactor
Langage dominant
Python
Étoiles
5
Forks
3
Métriques de merge des PR
Aucune PR mergée en 30 j

Description

Follow up ticket from standup discussion. There appears to be [existing code](https://github.com/AllenCell/allencell-ml-segmenter/blob/62b71091bda085dea68c2e4fd511561712e0072e/src/allencell_ml_segmenter/main/main_model.py#L23) for programmatic 'tab state' management in the main model (rather than having the tab state only exist in the main widget). But, it seems unused and is not kept in sync with the actual tab.

**TODO**:

- [ ] decide whether this refactor is worth it
- [ ] determine how to address the 'circular state' issue with having the `QTabWidget` respond to a change in the model state, but also cause that change in the model state

**From original comment**:

I think my ideal solution would look something like this (based on curation work + design conversations with Chris):

- main model would have `current_tab` state, which would maybe be an enum of `{Curation, Training, Prediction}`
- main model would have setter for `current_tab`, which would emit signal `current_tab_changed(TabTypeEnum)`
- main widget would call the setter when the tab focus is changed
- other widgets (curation, training) would have slots connected to the main model's `current_tab_changed` signal which can act accordingly (for curation, that would mean clearing current images if the new tab is not `Curation` or reloading them if the new tab is `Curation`, for instance)

The benefits here are that we would have a centralized piece of state that any view/model/service could look at and determine the current tab, and views could connect to the tab changed signal at will and do whatever they need to with that information. It would also align well with our goal of moving toward slot/signal design overall.

The downsides are **1** the `QTabWidget` design makes it so that we would have duplicate state for the current tab in main model / main widget (this is true right now as well, but these changes wouldn't solve it) and **2** it seems like a lot of work.

My vote is that we go with something similar to what you have here, so long as it works well. And then maybe we can make a ticket for what I've described in this comment. Lmk what you think.

**Note**: if we do stick with what you have here, we would need to call `focus_changed` on all widgets, not just the one we are shifting focus to.

_Originally posted by @saeliddp in https://github.com/AllenCell/allencell-ml-segmenter/issues/403#issuecomment-2176968658_

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.