NSLS2 / pymca

PyMca Toolkit git repository
Other
0 stars 3 forks source link

Add pytest unit tests for TiledDataSource #12

Closed padraic-shafer closed 1 month ago

padraic-shafer commented 1 month ago

Starting point for adding testable changes to PyMca toward meeting Milestone #1.

padraic-shafer commented 1 month ago

We've now learned enough about PyMca so that we can connect the various objects and signals to import data and plot it. We have also collected lot of artifacts that are distracting, which could interfere with the desired functionality as we go on. I created this milestone-1 branch so that we could have a fresh start.

I would like to see us decouple some of the responsibilities that are intertwined within QTiledWidget and QTiledDataSource. Something approaching a Model-View-ViewModel separation of responsibilities seems appropriate here, though we can evolve that approach if more (or fewer) layers are needed.

I would also like to see us develop this with a test-first approach (or at least keep pace between the tests and the functionality).


Here is a starting attempt at this...enough to paint a picture of where I think we should be headed. I've made a bare bones (although not yet minimal) version of the TiledDataSource (widget-free), a QTiledCatalogSelectorWidget as a start to the "Open connection..." dialog that we previously identified the need for, and a TiledCatalogSelector View Model that is meant to handle the "business logic" for the dialog. The logic of the model is independently testable and does not need to know about the Dialog widget. The dialog widget simply connects signals and slots with the model, and otherwise handles the on-screen aspects. There is still plenty left do here, including re-creating the data selector widget (QTiledWidget).

padraic-shafer commented 1 month ago

I should emphasize that there is still significant room to shape the design of our contributions. Toward that end, I'd really like to get feedback on how we could make this better.

padraic-shafer commented 1 month ago

To try and save 1000 words, here is a diagram. Currently I'm aiming for something like this.

image

@startuml

QTiledCatalogSelectorDialog -|> QDialog
QTiledCatalogSelectorDialog o-- "1" TiledCatalogSelector
TiledCatalogSelector - QDispatcher : NewSourceSelected >
QDispatcher -- TiledDataSource : create >
QDispatcher -- QTiledWidget : create >
TiledDataSource . QTiledWidget
QTiledWidget -|> QWidget
QTiledWidget o-- TiledDataSelector
TiledDataSource "1..n" -o TiledDataSelector

note left of QTiledCatalogSelectorDialog
  GUI only
end note

note left of TiledCatalogSelector
  * Get/Set dialog attributes
  * Interactions between attributes
end note

note right of TiledDataSelector
  Business logic
  for QTiledWidget
end note

@enduml
padraic-shafer commented 1 month ago

I've made changes in response to our discussion earlier today. Please let me know before Friday if you have additional suggestions. I'll plan to merge this on Friday morning.

Thank you!

padraic-shafer commented 1 month ago

@danielballan Thank you very much for your review. I'll make these updates over the next day.