pyopenms_viz icon indicating copy to clipboard operation
pyopenms_viz copied to clipboard

Ensure PeakMap log scaling applies only to colors, keeping marginal plots raw issue 65

Open Imama-Kainat opened this issue 9 months ago β€’ 8 comments

πŸ”Ή What This PR Fixes This PR fixes the issue where log scaling was incorrectly applied to both PeakMap colors and marginal plots. Now:

Log scaling is applied only to self.z for PeakMap visualization. βœ… Marginal plots always display raw intensity values (self.z_original). βœ… Avoids unintended log scaling side effects on other properties. βœ… πŸ”Ή Changes Made 1️⃣ Stored original intensity values before applying log transformation (self.z_original). 2️⃣ Updated plot_marginals() to always use self.z_original (raw intensities). 3️⃣ Ensured log scaling applies only to self.z (color mapping), not marginal plots.

Summary by CodeRabbit

  • New Features

    • Introduced an optional log-scale transformation for the z-axis, offering clearer visualization of intensity values.
    • Enhanced plot displays now include marginal distribution views utilizing original intensity details.
    • Added configuration options for controlling log scaling of colors and intensities in peak map plots.
    • New test suite added to validate plotting functionalities, ensuring correct handling of log scaling and original intensity values.
  • Bug Fixes

    • Improved handling of marginal plots to ensure they utilize original intensity values even when log scaling is applied.

Imama-Kainat avatar Mar 15 '25 02:03 Imama-Kainat

Walkthrough

The changes refactor the PeakMapPlot class in the pyopenms_viz/_core.py module. A new constructor introduces an optional z_log_scale parameter, and a prepare_data method is added to manage intensity and z-value transformations, including applying logarithmic scaling. The plot method now integrates logic to include marginal distributions, calling a new method create_main_plot_marginals when needed. The PeakMapConfig class in pyopenms_viz/_config.py is updated with new attributes to control log scaling for colors and intensities. Additionally, modifications are made in the BOKEHPeakMapPlot and MATPLOTLIBPeakMapPlot classes to enhance plotting functionalities.

Changes

File Path Change Summary
pyopenms_viz/_core.py - Updated PeakMapPlot constructor to accept an optional z_log_scale parameter.
- Added prepare_data method for intensity conversion and z-value handling.
- Modified plot method to support plotting marginal distributions and use original intensity values when available.
- Added create_main_plot_marginals method.
pyopenms_viz/_config.py - Added attributes z_log_scale_colors (default True) and z_log_scale_intensity (default False) in PeakMapConfig.
- Updated __post_init__ for marginal plots and axis labels.
- Modified legend configuration for y-axis marginal plots.
pyopenms_viz/_bokeh/core.py - Changed create_main_plot method to a standalone function with enhanced functionality for PeakMap plotting.
- Added create_x_axis_plot and create_y_axis_plot methods.
pyopenms_viz/_matplotlib/core.py - Updated create_x_axis_plot, create_y_axis_plot, and create_main_plot methods to utilize z_original for marginal histograms and apply log scaling for color mapping.
test/peaktest.py - Introduced a new test suite with tests for the plot method and marginal plots, validating log scaling and original intensity values.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Plot as PeakMapPlot
    User->>Plot: Instantiate (optionally with z_log_scale=True)
    Note over Plot: Stores original intensity if z_log_scale is True
    User->>Plot: Call prepare_data()
    Plot->>Plot: Convert intensity to relative values & bin peaks\nApply log transformation on z if required
    User->>Plot: Call plot()
    Plot->>Plot: Ensure data is prepared\nCheck for 'add_marginals'
    alt Marginals Required
        Plot->>Plot: Call create_main_plot_marginals()
    end

Poem

Oh, how I hop with code so sweet,
Transforming plots with a rhythmic beat.
Logarithms and data neatly binned,
Marginal plots join the joyful spin.
A rabbit's cheer in every line we greet! πŸ‡βœ¨

[!TIP]

⚑πŸ§ͺ Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments. - To enable this feature, set early_access to true under in the settings.
✨ Finishing Touches
  • [ ] πŸ“ Generate Docstrings

πŸͺ§ Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Mar 15 '25 02:03 coderabbitai[bot]

@maintainers I haven’t completed 100% testing yet, so please hold off on reviewing it for now.

Imama-Kainat avatar Mar 15 '25 02:03 Imama-Kainat

Have not done a complete review yet however, it seems that we are getting some failed tests. I expect some to fail since you are updating the code however I am concerned with the documentation not building. Can you please look into this?

jcharkow avatar Mar 15 '25 13:03 jcharkow

Hey @t0mdavid-m @jcharkow ,

I have made the following improvements to the PeakMap plotting logic:

βœ… Applied log scaling only to color mapping** (z values) to prevent unintended intensity distortions.
βœ… Ensured marginal histograms (x and y) use raw intensity (z_original)** for correct visualization.
βœ… Adjusted axis ranges in create_x_axis_plot() and create_y_axis_plot()** to maintain proper alignment with the main PeakMap plot.

Please review and let me know if any modifications are needed.

Thanks!

Imama-Kainat avatar Mar 15 '25 17:03 Imama-Kainat

Summary of Changes & Fixes

πŸ”Ή Log Scaling Applied Only to Colors

Previously, log scaling affected the entire dataset, distorting histograms. Fix: Log transformation (np.log1p) is now only applied to color mapping (z), keeping raw values intact. πŸ”Ή Marginal Histograms Use z_original

X and Y histograms should display raw intensity values, not log-transformed ones. Fix: Used z_original for histograms, ensuring correct intensity representation.

Imama-Kainat avatar Mar 15 '25 17:03 Imama-Kainat

@Imama-Kainat can you please link to the issue this is solving (e.g. edit the description and add the issue that this addresses)

jcharkow avatar Mar 15 '25 17:03 jcharkow

image my log scaling solution flow

Imama-Kainat avatar Mar 15 '25 18:03 Imama-Kainat

  • can you resolve merge issues
  • can you post several example images here?

timosachsenberg avatar Mar 24 '25 08:03 timosachsenberg