AllenCell / AllenCell/allencell-segmenter-ml

Refactor state management for main tabs

Aperta
#404 0 commenti 0 reazioni 3 assegnatari Rivendicata da @yrkim98 Vedi su GitHub
refactor
Lingua principale
Python
Stelle
5
Fork
3
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

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_

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.