napari-clusters-plotter icon indicating copy to clipboard operation
napari-clusters-plotter copied to clipboard

WIP: Restructuring plotter

Open zoccoler opened this issue 1 year ago • 6 comments
trafficstars

Now that I am back, I'd like to slowly continue this.

This is waaaay far from merging, but I am opening it as Draft to have a way to refer to changes, etc

@Cryaaa, @jo-mueller, is this branch the most up-to-date or is it the Plotter_Code_Overhaul branch?

@Cryaaa , I was trying to give it a test, but I am not able to run it, I get this error (I updated the link to the .ui file to my local file)

File [c:\users\mazo260d\documents\github\napari-clusters-plotter\napari_clusters_plotter\new_plotter_widget.py:97](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:97), in PlotterWidget.__init__(self, napari_viewer)
     [94](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:94) # Setting of Widget options
     [95](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:95) self.hue: QComboBox = self.control_widget.hue_box
---> [97](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:97) self.import_feats: QPushButton = self.control_widget.feature_import_button
     [98](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:98) self.update_button: QPushButton = self.control_widget.update_axes_button
     [99](file:///C:/users/mazo260d/documents/github/napari-clusters-plotter/napari_clusters_plotter/new_plotter_widget.py:99) self.plot_button: QPushButton = self.control_widget.feature_import_button

AttributeError: 'QWidget' object has no attribute 'feature_import_button'

What am I missing? Should I pursue working on this branch?

zoccoler avatar Feb 26 '24 11:02 zoccoler

Codecov Report

Attention: Patch coverage is 0% with 118 lines in your changes missing coverage. Please review.

Project coverage is 75.21%. Comparing base (ba7924d) to head (b0f831c).

Files Patch % Lines
napari_clusters_plotter/new_plotter_widget.py 0.00% 118 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #305      +/-   ##
==========================================
- Coverage   79.49%   75.21%   -4.29%     
==========================================
  Files          16       17       +1     
  Lines        2073     2191     +118     
==========================================
  Hits         1648     1648              
- Misses        425      543     +118     

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

codecov[bot] avatar Feb 26 '24 11:02 codecov[bot]

Hey @zoccoler! This was my attempt at implementing the Qt designer into the plotter widget. It is still in its baby steps but maybe it would make sense to use this branch. If you want to continue working on this we could have a meeting in person or over zoom so I can explain some of my rationale behind this branch? As for the error I think I have found the source of it already and will fix it.

Cryaaa avatar Feb 26 '24 11:02 Cryaaa

Hey, a quick zoom meeting sounds like a god idea so that I can catch up! I will send you an email.

The bug is not that one, I had that path fixed already, it comes from this line.

zoccoler avatar Feb 26 '24 15:02 zoccoler

Hi @Cryaaa ! I am putting the new basic plotting related things in another repository biaplotter to simplify things here. The napari-clusters-plotter fields would then be built on top of that

zoccoler avatar Mar 27 '24 11:03 zoccoler

Hey @zoccoler, that sounds good to me, makes this codebase a bit more lightweight!

Cryaaa avatar Mar 27 '24 12:03 Cryaaa

Hi everyone,

I just released a first version of the biaplotter, hopefully it will help simplify the code structure here. Please take a look at the repository, I wrote tests, examples and API documentation.

It has independent artists and selector classes that connect to each other via signals and slots. The main class is called CanvasWidget and that is the one I think we should use/subclass here.

If you feel something is missing that should belong there, feel free to create issues and PRs.

Best, Marcelo

zoccoler avatar May 08 '24 07:05 zoccoler

@zoccoler I made some progress on this one and I think the only thing left to do (as far as I can see) are

  • hooking up some of the functionalities already in the widget (changing plot type to histogram, setting the color of the scatterplot)
  • Respecting the timelapse-dimension if there is one

jo-mueller avatar Jul 18 '24 15:07 jo-mueller

I continue the work in this branch - I think this PR could be closed.

jo-mueller avatar Jul 23 '24 11:07 jo-mueller

I continue the work in this branch - I think this PR could be closed.

Wow thanks for all of the work you put in! Agree to change to a new branch which has more structure and I guess we can open a new PR as we move closer to adding in all of the updates.

Cryaaa avatar Jul 23 '24 12:07 Cryaaa