Ensure PeakMap log scaling applies only to colors, keeping marginal plots raw issue 65
πΉ 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.
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
@coderabbitaiin 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
@coderabbitaiin 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 pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate docstringsto generate docstrings for this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile 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.
@maintainers I havenβt completed 100% testing yet, so please hold off on reviewing it for now.
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?
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!
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 can you please link to the issue this is solving (e.g. edit the description and add the issue that this addresses)
my log scaling solution flow
- can you resolve merge issues
- can you post several example images here?