refactor underlying architecture of plugin for molularity and encapsulation

alessandrofelder commented 10 months ago

Description

What is this PR

[ ] Bug fix
[ ] Addition of a new feature
[x] Other

Why is this PR needed? The atlas_viewer_widget.py file was becoming unwieldy to add functionality to, and things that were napari-independent were tested in a napari context as a consequence of the previous architecture (which made the tests slow, as well as violating separation of concerns).

What does this PR do?

This PR splits the classes more independent and have clearer responsibilities. Some things have been renamed for clarity too. The plugin should look and behave exactly the same as before (bar also fixing #66 ) a key advantage is better modularity

widgets can be used and tested independent of napari (and the tests seem faster to run?)
each file and class has clear responsibilities
easier to add new features to because the amount of context needed to worry about at once is reduced

a downside is that there is an extra layer of abstraction in the code(but I think it's worth it!)

References

Closes #8 Closes #66

How has this PR been tested?

Tests have been refactored accordingly, and pass. Also extensive manual testing for any unintentional changes in behaviour.

Is this a breaking change?

Not for the user, big change in API which is good to be done early in the development process for this plugin

Does this PR require an update to the documentation?

Will be added as part of #52. Docstrings improved and added as needed by the refactoring.

Checklist:

[x] The code has been tested locally
[x] Tests have been added to cover all new functionality (unit & integration)
[x] The documentation has been updated to reflect any changes
[x] The code has been formatted with pre-commit

codecov[bot] commented 10 months ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +5.47% :tada:

Comparison is base (76733ed) 91.85% compared to head (232371b) 97.32%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #67 +/- ## ========================================== + Coverage 91.85% 97.32% +5.47% ========================================== Files 6 7 +1 Lines 270 299 +29 ========================================== + Hits 248 291 +43 + Misses 22 8 -14 ``` | [Files Changed](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe) | Coverage Δ | | |---|---|---| | [brainrender\_napari/utils/load\_user\_data.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL3V0aWxzL2xvYWRfdXNlcl9kYXRhLnB5) | `100.00% <ø> (ø)` | | | [...rainrender\_napari/widgets/atlas\_download\_dialog.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL3dpZGdldHMvYXRsYXNfZG93bmxvYWRfZGlhbG9nLnB5) | `100.00% <ø> (ø)` | | | [brainrender\_napari/\_\_init\_\_.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [brainrender\_napari/brainrender\_widget.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL2JyYWlucmVuZGVyX3dpZGdldC5weQ==) | `100.00% <100.00%> (ø)` | | | [brainrender\_napari/widgets/atlas\_table\_view.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL3dpZGdldHMvYXRsYXNfdGFibGVfdmlldy5weQ==) | `100.00% <100.00%> (ø)` | | | [brainrender\_napari/widgets/structure\_view.py](https://app.codecov.io/gh/brainglobe/brainrender-napari/pull/67?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=brainglobe#diff-YnJhaW5yZW5kZXJfbmFwYXJpL3dpZGdldHMvc3RydWN0dXJlX3ZpZXcucHk=) | `92.92% <100.00%> (ø)` | |

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

alessandrofelder commented 10 months ago

I am going to fix the slight decrease in coverage, but in the meantime it would be great if, @adamltyson, you could check that you agree that nothing is broken for the user by this PR (it's supposed to be a pure moving-stuff-around and communicate-via-qt-signals-instead PR) with no changes in functionality for the user?

brainglobe / brainrender-napari