Add/3d peakmap

[singjc]

User description

I added 3D peak map plotting for plotly and matplotlib. I did not implement one for bokeh, because I previously didn't find a good way to create 3D vertical lines plots for bokeh.

---

PR Type

enhancement, bug fix

---

Description

• Introduced 3D plotting capabilities for Plotly and Matplotlib, allowing for enhanced visualization of data.
• Added a plot_3d parameter across multiple modules to toggle between 2D and 3D plotting.
• Improved color handling and layout settings for 3D plots in Plotly.
• Implemented checks for unsupported 3D plots in Bokeh, ensuring graceful handling.
• Updated configuration settings to include zlabel for 3D plots.

---

Changes walkthrough 📝

	Relevant files
Enhancement	core.py Implement 3D plotting functionality in Plotly                        pyopenms_viz/_plotly/core.py Added 3D plotting capabilities for Plotly, including vertical line plots. Introduced a plot_3d parameter to toggle 3D plotting. Enhanced color handling for line plots. Updated layout and camera settings for 3D plots. +157/-38 core.py Add 3D plotting support in Matplotlib                                        pyopenms_viz/_matplotlib/core.py Added 3D plotting support for Matplotlib. Introduced a plot_3d parameter to toggle 3D plotting. Enhanced axis and grid settings for 3D plots. +97/-26  _core.py Extend core plotting functions for 3D support                        pyopenms_viz/_core.py Added plot_3d and zlabel parameters to core plotting functions. Updated plot generation to support 3D plots. Improved data binning and grouping logic. +44/-10  core.py Add 3D plot parameter and checks in Bokeh                                pyopenms_viz/_bokeh/core.py Added plot_3d parameter to plotting functions. Implemented a check for unsupported 3D plots in Bokeh. +38/-32
Configuration changes	_config.py Update plot configuration for 3D support                                  pyopenms_viz/_config.py Added zlabel configuration for 3D plots. Updated plot label settings for peak maps. +4/-0
Additional files (token-limit)	PeakMap.ipynb ...                                                                                                            nbs/PeakMap.ipynb ... +591/-33 pyopenms_viz_tutorial.ipynb ...                                                                                                            nbs/pyopenms_viz_tutorial.ipynb ... +1/-905

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Potential Performance Issue The PeakMapPlot class is grouping data and calculating mean intensity within each bin. This operation could be computationally expensive for large datasets and might need optimization. Code Duplication There's significant code duplication in the plot method of PLOTLYVLinePlot class for 2D and 3D plotting. Consider refactoring to reduce redundancy. Hardcoded Values The _create_figure method contains hardcoded values for font sizes, colors, and layout parameters. These should be configurable or moved to a constants file.

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Refactor the plot method to separate 2D and 3D plotting logic The plot method for BOKEHVLinePlot contains a large conditional block for 2D and 3D plotting. Consider splitting this into separate methods for 2D and 3D plotting to improve readability and maintainability. pyopenms_viz/_bokeh/core.py [256-288] -if not plot_3d: +def plot_2d(fig, data, x, y, by, **kwargs): if by is None: source = ColumnDataSource(data) - line = fig.segment( - x0=x, - y0=0, - x1=x, - y1=y, - source=source, - line_color="line_color", - **kwargs, - ) + line = fig.segment(x0=x, y0=0, x1=x, y1=y, source=source, line_color="line_color", **kwargs) return fig, None else: legend_items = [] for group, df in data.groupby(by): source = ColumnDataSource(df) - line = fig.segment( - x0=x, - y0=0, - x1=x, - y1=y, - source=source, - line_color="line_color", - **kwargs, - ) + line = fig.segment(x0=x, y0=0, x1=x, y1=y, source=source, line_color="line_color", **kwargs) legend_items.append((group, [line])) + return fig, Legend(items=legend_items) - legend = Legend(items=legend_items) - - return fig, legend +if not plot_3d: + return plot_2d(fig, data, x, y, by, **kwargs) else: raise NotImplementedError("3D Vline plots are not supported in Bokeh") Apply this suggestion Suggestion importance[1-10]: 8 Why: Splitting the plotting logic into separate methods for 2D and 3D enhances readability and maintainability, making the code structure clearer and easier to extend or modify in the future.	8
	Extract 3D plot configuration into a separate method for better code organization Consider extracting the 3D plot configuration into a separate method to improve code organization and reusability. This will make the code more maintainable and easier to understand. pyopenms_viz/_matplotlib/core.py [67-81] self.fig = self.superFig.add_subplot(111, projection="3d") -self.fig.set_title(self.title) -self.fig.set_xlabel(self.xlabel, - fontsize=9, - labelpad=-2, - color=ColorGenerator.color_blind_friendly_map[ColorGenerator.Colors.DARKGRAY], - style="italic") -self.fig.set_ylabel(self.ylabel, - fontsize=9, - labelpad=-2, - color=ColorGenerator.color_blind_friendly_map[ColorGenerator.Colors.DARKGRAY]) -self.fig.set_zlabel(self.zlabel, - fontsize=10, - color=ColorGenerator.color_blind_friendly_map[ColorGenerator.Colors.DARKGRAY], - labelpad=-2) +self._configure_3d_plot() Apply this suggestion Suggestion importance[1-10]: 7 Why: Extracting the 3D plot configuration into a separate method enhances code organization and reusability, which is beneficial for maintainability.	7
	Refactor data grouping logic into a separate method to reduce code duplication The grouping logic for data with and without the 'by' parameter is very similar. Consider refactoring this into a separate method to reduce code duplication and improve maintainability. pyopenms_viz/_core.py [838-854] +def group_data(data, x, y, z, by=None): + group_cols = [x, y] + ([by] if by else []) + return data.groupby(group_cols, observed=True).agg({z: "mean"}).reset_index() + by = kwargs.pop("by", None) +data = group_data(data, x, y, z, by) if by is not None: - # Group by x, y and by columns and calculate the mean intensity within each bin - data = ( - data.groupby([x, y, by], observed=True) - .agg({z: "mean"}) - .reset_index() - ) - # Add by back to kwargs kwargs["by"] = by -else: - # Group by x and y bins and calculate the mean intensity within each bin - data = ( - data.groupby([x, y], observed=True) - .agg({z: "mean"}) - .reset_index() - ) Apply this suggestion Suggestion importance[1-10]: 7 Why: Refactoring the grouping logic into a separate method reduces code duplication and enhances maintainability, making the code cleaner and easier to manage.	7
	Use a more descriptive variable name for the third dimension in 3D plots Consider using a more descriptive variable name instead of 'z' for the third dimension in the 3D plot. This will improve code readability and make it clearer what the variable represents. pyopenms_viz/_plotly/core.py [289-290] -if 'z' in kwargs: - z = kwargs.pop('z') +if 'intensity' in kwargs: + intensity = kwargs.pop('intensity') Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion to use a more descriptive variable name improves code readability and maintainability, but it is not critical to the functionality of the code.	6
	Extract color generation logic into a separate method for better reusability The color generation logic is repeated in multiple places. Consider extracting this into a separate method to improve code reusability and maintainability. pyopenms_viz/_bokeh/core.py [221-222] -if color_gen is not None: - kwargs["line_color"] = color_gen if isinstance(color_gen, str) else next(color_gen) +def get_line_color(color_gen): + return color_gen if isinstance(color_gen, str) else next(color_gen) if color_gen else None +kwargs["line_color"] = get_line_color(color_gen) + Apply this suggestion Suggestion importance[1-10]: 6 Why: Extracting the color generation logic into a separate method improves code reusability and maintainability, although the improvement is minor since the logic is simple.	6
Possible issue	Add error handling for missing required parameters in 3D plotting Consider adding error handling for the case when 'z' is not present in kwargs. This will make the code more robust and prevent potential runtime errors. pyopenms_viz/_plotly/core.py [289-290] if 'z' in kwargs: z = kwargs.pop('z') +else: + raise ValueError("'z' parameter is required for 3D plotting") Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding error handling for missing parameters is important for robustness and preventing runtime errors, making this a valuable improvement.	8
Best practice	Use constants for color values instead of hardcoding them Consider using a constant or configuration variable for color values instead of hardcoding them. This will make it easier to maintain and update the color scheme across the application. pyopenms_viz/_matplotlib/core.py [88] -self.fig.grid(color="#FF0000", linewidth=0.8) +GRID_COLOR = "#FF0000" +GRID_LINEWIDTH = 0.8 +self.fig.grid(color=GRID_COLOR, linewidth=GRID_LINEWIDTH) Apply this suggestion Suggestion importance[1-10]: 5 Why: Using constants for color values is a good practice for maintainability, but it is a minor improvement that does not affect the core functionality of the code.	5
Enhancement	Simplify color generation logic by using a default value Consider using a default value for color_gen instead of checking if it's None. This can simplify the code and make it more readable. pyopenms_viz/_core.py [498-501] -if color_gen is not None: - kwargs["line_color"] = color_gen if isinstance(color_gen, str) else next(color_gen) +kwargs["line_color"] = color_gen if isinstance(color_gen, str) else next(color_gen) if color_gen else None Apply this suggestion Suggestion importance[1-10]: 4 Why: The suggestion simplifies the code slightly by removing the explicit check for None, but it doesn't significantly improve readability or maintainability. The existing logic is already clear and concise.	4

[axelwalter]

Great!