Skip to content

Conversation

srivarra
Copy link
Contributor

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Addresses a few incorrect and missing features in the mass plotting notebook.

  • Adds dpi and fig_size to both cohort_cluster_plot and color_segmentation_by_stat
  • Fixes incorrect sorting of colors when the colormap file is provided in cohort_cluster_plot
  • Adds interpolation "none" to color_segmentation_by_stat to prevent interpolation of the cell borders
  • Incorrect colors are properly reported back to the user
  • Adds an option to save the plot as any other format accepted by matplotlib

How did you implement your changes

Added those small changes. The reason the colors were not mapping to the cluster appropriately was because the NumPy Array conversion to the colormaps didn't take into account that the colors need to be sorted w.r.t their associated cluster id.

Remaining issues

N/A.

@srivarra srivarra linked an issue Sep 27, 2023 that may be closed by this pull request
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@srivarra srivarra self-assigned this Sep 27, 2023
@srivarra srivarra added the bug Something isn't working label Sep 27, 2023
Copy link
Contributor

@camisowers camisowers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Just one clarifying question.

@srivarra srivarra marked this pull request as ready for review September 27, 2023 21:40
@camisowers camisowers self-requested a review September 27, 2023 21:49
Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@srivarra srivarra requested a review from ngreenwald September 28, 2023 17:55
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@ngreenwald ngreenwald added this pull request to the merge queue Sep 29, 2023
Merged via the queue into main with commit ce642b0 Sep 29, 2023
@ngreenwald ngreenwald deleted the mass_plotting/fixes branch September 29, 2023 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mass Plotting needs a few adjustments
4 participants