Skip to content

fix(dataProcessPlots): Fix error bar caps / whiskers for profile plots for plotly visualization#200

Merged
tonywu1999 merged 1 commit intodevelfrom
fix-profile-plots
Apr 23, 2026
Merged

fix(dataProcessPlots): Fix error bar caps / whiskers for profile plots for plotly visualization#200
tonywu1999 merged 1 commit intodevelfrom
fix-profile-plots

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented Apr 23, 2026

PR Type

Bug fix


Description

  • Fix Plotly error bar caps

  • Apply cap widths to profile plots

  • Update original and summary conversions


Diagram Walkthrough

flowchart LR
  ggplot["GGPlot profile plots"]
  convert["Convert to Plotly"]
  legend["Legend and censored point fixes"]
  caps["Set error bar cap width"]
  output["Rendered Plotly profile plots"]

  ggplot -- "converted" --> convert
  convert -- "post-process" --> legend
  legend -- "apply" --> caps
  caps -- "improves" --> output
Loading

File Walkthrough

Relevant files
Bug fix
dataProcessPlots.R
Add Plotly error bar cap correction                                           

R/dataProcessPlots.R

  • Add .fixErrorBarCapsPlotly() helper
  • Set error_y$width for Plotly traces
  • Apply fix to original_plot conversions
  • Apply fix to summary_plot conversions
+11/-0   

Motivation and Context

Profile plots are used in MSstats to identify potential sources of variation for each protein in mass spectrometry data. When using Plotly as the visualization backend (instead of ggplot2/PDF), error bar caps and whiskers were not being rendered with the correct width, leading to inconsistent visual representation of confidence intervals and standard deviations. This fix ensures that error bar cap widths are set consistently across all Plotly-based profile plot renderings.

Changes

  • Added new internal helper function .fixErrorBarCapsPlotly() that post-processes Plotly plot objects to ensure error bar cap widths are configured correctly
    • The function accepts a plot object and an optional cap_width parameter (defaults to 8)
    • Iterates through all traces in the plot's data structure (plot$x$data)
    • For each trace containing vertical error bars (error_y object), sets the error_y$width property to the specified cap width
    • Returns the modified plot object
  • Applied this post-processing helper to both original and summary profile plot renderings when converting from ggplot2 to Plotly (lines 158 and 172 of R/dataProcessPlots.R)
  • The fix is integrated into the existing pipeline that already applies other Plotly-specific adjustments such as legend fixes and censored points handling
  • No other plotting modes or non-Plotly code paths were modified

Testing

No new unit tests were added or modified. The change applies only to internal post-processing functions and does not alter the external API or function signatures.

Coding Guidelines

No coding guideline violations detected. The implementation follows the existing patterns established by similar post-processing helper functions in the codebase (e.g., .fixLegendPlotlyPlotsDataprocess(), .fixCensoredPointsLegendProfilePlotsPlotly()), maintains consistent naming conventions with the dot-prefix for internal utilities, and is properly integrated into the existing visualization pipeline.

@tonywu1999 tonywu1999 changed the title fix(dataProcessPlots): Fix error bar caps / whiskers for profile plots fix(dataProcessPlots): Fix error bar caps / whiskers for profile plots for plotly visualization Apr 23, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

📝 Walkthrough

Walkthrough

A new .fixErrorBarCapsPlotly() helper function was added to post-process Plotly-generated profile plot figures. The helper iterates over plot traces and sets error bar cap width to a configurable value (defaulting to 8) for traces containing error bars.

Changes

Cohort / File(s) Summary
Error Bar Cap Post-Processing
R/dataProcessPlots.R
Added .fixErrorBarCapsPlotly() helper function to standardize error bar cap width across Plotly profile plot renderings, affecting both original and run-summary plot outputs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested labels

Review effort 1/5

Suggested reviewers

  • devonjkohler

Poem

🐰 A helper so neat, from the coding warren,
Fixes error bar caps with config to follow,
Plotly's traces now perfectly shallow,
Width of eight, consistent and narrow,
One little function, yet oh how it's narrow! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description lacks required sections from the template. While it includes PR Type and a creative diagram, the Motivation and Context, Changes, and Testing sections are either missing or insufficiently detailed. Add detailed Motivation and Context explaining the error bar cap issue, expand the Changes section with specific code modifications, and document any testing performed. Complete the pre-review checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing error bar caps/whiskers for profile plots in plotly visualization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-profile-plots

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle both error-bar axes

This only updates error_y, so plots rendered with flipped axes or horizontal error
bars will still miss cap widths. Update both error_y and error_x so the fix works
regardless of how ggplotly encodes the error bars.

R/dataProcessPlots.R [701-708]

 .fixErrorBarCapsPlotly = function(plot, cap_width = 8) {
     for (i in seq_along(plot$x$data)) {
         if (!is.null(plot$x$data[[i]]$error_y)) {
             plot$x$data[[i]]$error_y$width <- cap_width
         }
+        if (!is.null(plot$x$data[[i]]$error_x)) {
+            plot$x$data[[i]]$error_x$width <- cap_width
+        }
     }
     plot
 }
Suggestion importance[1-10]: 5

__

Why: Updating only error_y leaves horizontal or flipped-axis error bars untouched, so extending .fixErrorBarCapsPlotly to also set error_x$width is a valid robustness improvement. The suggestion matches the added helper and is low risk, but its impact is moderate because these plots may still use only error_y.

Low
General
Fix final trace set

Applying the cap-width fix before .retainCensoredDataPoints can miss traces that are
added or rebuilt in that later step. Run .fixErrorBarCapsPlotly after the
conditional transformation so the final plot$x$data always gets updated; the same
reorder should be applied in the summary_plot branch too.

R/dataProcessPlots.R [157-161]

 og_plotly_plot = .fixCensoredPointsLegendProfilePlotsPlotly(og_plotly_plot)
-og_plotly_plot = .fixErrorBarCapsPlotly(og_plotly_plot)
 
 if(toupper(featureName) == "NA") {
     og_plotly_plot = .retainCensoredDataPoints(og_plotly_plot)
 }
+og_plotly_plot = .fixErrorBarCapsPlotly(og_plotly_plot)
Suggestion importance[1-10]: 3

__

Why: Moving .fixErrorBarCapsPlotly after .retainCensoredDataPoints could help if .retainCensoredDataPoints rebuilds plot$x$data, but that behavior is not demonstrated in the diff. The suggestion is plausible, yet somewhat speculative and limited to the featureName == "NA" path.

Low

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
R/dataProcessPlots.R (1)

701-708: LGTM — small helper, correct and narrowly scoped.

The guard against NULL error_y is appropriate, and fixing the cap width post-ggplotly conversion is a reasonable workaround for the known whisker-width issue in plotly's ggplot converter.

Two optional considerations:

  • cap_width is plumbed as a parameter here but not exposed through dataProcessPlots() / .plotProfile(), so callers can never override the default. If you want this to be tunable, thread it through; otherwise, consider inlining the constant to avoid a dead parameter.
  • Only error_y$width is set. If a future plot ever uses horizontal error bars (error_x), caps there will still be off. A defensive symmetric fix is cheap:
♻️ Optional: also normalize error_x cap width
 .fixErrorBarCapsPlotly = function(plot, cap_width = 8) {
     for (i in seq_along(plot$x$data)) {
         if (!is.null(plot$x$data[[i]]$error_y)) {
             plot$x$data[[i]]$error_y$width <- cap_width
         }
+        if (!is.null(plot$x$data[[i]]$error_x)) {
+            plot$x$data[[i]]$error_x$width <- cap_width
+        }
     }
     plot
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@R/dataProcessPlots.R` around lines 701 - 708, .fixErrorBarCapsPlotly
currently accepts cap_width that callers cannot override and only adjusts
vertical error caps (error_y); either remove cap_width and inline the chosen
constant or thread cap_width through the public plotting entrypoints
(dataProcessPlots and .plotProfile) so callers can pass a value, and update
.fixErrorBarCapsPlotly to also set plot$x$data[[i]]$error_x$width when present
(in addition to error_y$width) to defensively handle horizontal error bars.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@R/dataProcessPlots.R`:
- Around line 701-708: .fixErrorBarCapsPlotly currently accepts cap_width that
callers cannot override and only adjusts vertical error caps (error_y); either
remove cap_width and inline the chosen constant or thread cap_width through the
public plotting entrypoints (dataProcessPlots and .plotProfile) so callers can
pass a value, and update .fixErrorBarCapsPlotly to also set
plot$x$data[[i]]$error_x$width when present (in addition to error_y$width) to
defensively handle horizontal error bars.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 25ac8c9f-fc82-43b6-a0f0-2997e1862dc9

📥 Commits

Reviewing files that changed from the base of the PR and between fb0fcf4 and f39b4fd.

📒 Files selected for processing (1)
  • R/dataProcessPlots.R

@tonywu1999 tonywu1999 merged commit cf46505 into devel Apr 23, 2026
3 checks passed
@tonywu1999 tonywu1999 deleted the fix-profile-plots branch April 23, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant