Skip to content

Refactor some functions in histogram tools#395

Closed
jo-mueller wants to merge 7 commits intoconnect_histogram_optional_featuresfrom
refactor-some-functions-in-histogram-tools
Closed

Refactor some functions in histogram tools#395
jo-mueller wants to merge 7 commits intoconnect_histogram_optional_featuresfrom
refactor-some-functions-in-histogram-tools

Conversation

@jo-mueller
Copy link
Collaborator

@jo-mueller jo-mueller commented May 6, 2025

@zoccoler small refactoring changes to #390. I changed the baplotter dependency to the branch on the repo to make sure that tests can pass, will change it as soon as 0.3.1 is released.

PS: I tested the functionality manually and everything seems to work. Tests are passing locally.

Long copilot description

This pull request refactors the color management logic in the napari_clusters_plotter widget to improve clarity and maintainability. Key changes include replacing redundant methods, simplifying color application, and introducing a new utility function for layer color updates.

Refactoring and Simplification:

  • Replaced the _on_show_plot_overlay method with a direct connection to _update_layer_colors, removing unnecessary indirection in handling the show_color_overlay_signal. [1] [2]
  • Simplified the _update_layer_colors method by consolidating logic for applying default and indexed colors, and removing redundant code paths.

Utility Function Addition:

  • Introduced a new _apply_layer_color utility function to centralize the logic for applying colors to layers, improving code reuse and readability.

Code Cleanup:

  • Removed the _set_layer_color method and inlined its functionality into _apply_layer_color, as it was no longer needed.
  • Eliminated the _reset method from its previous location and moved its functionality into a new definition, aligning it with the refactored _update_layer_colors logic. [1] [2]

@zoccoler zoccoler mentioned this pull request May 6, 2025
5 tasks
@zoccoler
Copy link
Collaborator

zoccoler commented May 6, 2025

Hi @jo-mueller ,

That's great! Thinks seem to be working! I found only one new bug:

When I draw a cluster, then change the Hue to something non-categorical, and draw again, the MANUAL_CLUSTER_ID is brought back correctly, but my first selection is lost.

@jo-mueller
Copy link
Collaborator Author

jo-mueller commented May 8, 2025

Argh ok if I'm only making it worse with my refactorings then maybe we should just merge #390 and then take it from there 😅.

@jo-mueller jo-mueller closed this May 8, 2025
@jo-mueller jo-mueller deleted the refactor-some-functions-in-histogram-tools branch May 8, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants