Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Restructuring plotter #305

Closed
wants to merge 41 commits into from
Closed

WIP: Restructuring plotter #305

wants to merge 41 commits into from

Conversation

zoccoler
Copy link
Collaborator

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 and others added 3 commits November 30, 2023 16:25
- plotter.py with:
  - custom scatter class
  - custom lasso class
  - plotter class derived from SingleAxesWidget class from napari-matplotlib
  - layer controls from .ui file created with designer
- plotter_controls.ui file
- notebook to test plotter with synthetic data
@zoccoler zoccoler added the work in progress Discussion on currently worked on issues label Feb 26, 2024
Copy link

codecov bot commented Feb 26, 2024

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 ⚠️
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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Cryaaa
Copy link
Collaborator

Cryaaa commented Feb 26, 2024

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.

@zoccoler
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

@Cryaaa
Copy link
Collaborator

Cryaaa commented Mar 27, 2024

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

@zoccoler
Copy link
Collaborator Author

zoccoler commented May 8, 2024

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

@jo-mueller
Copy link
Collaborator

@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
Copy link
Collaborator

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

@Cryaaa
Copy link
Collaborator

Cryaaa commented Jul 23, 2024

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.

@jo-mueller jo-mueller closed this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress Discussion on currently worked on issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants