Investigate CI failure in pyopenms-docs PR

[timosachsenberg]

The CI failure in PR #480 was caused by broken indentation in interactive_plots.rst. The hd.dynspread(...) block was at column 0 instead of being indented with 4 spaces, placing it outside the RST code-block directive.

When notebooks are generated from this RST file, the unindented code becomes markdown text instead of Python code, causing notebook execution to fail.

This commit applies the PR #480 changes (PeptideIdentificationList, get2DPeakDataLong 5th arg, refactored opts) with correct indentation.

Summary by CodeRabbit

• Documentation

  • Updated interactive plots guide with a new low-intensity peak filtering example and revised usage patterns.

• Refactor

  • Simplified plotting examples by applying visualization options directly via chaining for clearer, more consistent renders.
  • Peak data extraction example updated to include an additional parameter to improve 2D peak retrieval behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Documentation example updated: removed sys import, added a ThresholdMower low-intensity peak filter (threshold set to 5000.0), get2DPeakDataLong call extended with a fifth argument 1, and plotting options moved to a chained .opts(...) on the raster result.

Changes

Cohort / File(s)	Change Summary
Documentation example docs/source/user_guide/interactive_plots.rst	Removed import sys; added ThresholdMower-based low-intensity peak filter (threshold_filter, defaults, threshold = 5000.0, applyFilterPeakMap(exp)); updated get2DPeakDataLong(..., 1) call (added fifth arg 1); replaced inline plot=dict(...) usage with chained .opts(width=800, height=800, xlabel=..., ylabel=...) on the raster and kept dynspread chaining.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

• Verify the documentation snippet runs as-is (imports present, names match).
• Confirm the chained .opts(...) call syntax matches the plotting library version used in the docs.

Poem

A rabbit tweaks the docs with cheer, 🐇
A threshold set, a plot made clear.
One extra arg, a tidy chain,
Peaks trimmed down — a gentle gain.
Hops, ink, and code — a tiny cheer. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The PR title 'Investigate CI failure in pyopenms-docs PR' is vague and generic, using the non-descriptive term 'Investigate CI failure' without explaining what the actual fix or changes are.	Retitle to be more specific about the fix, such as 'Fix indentation in interactive_plots.rst code block' or 'Correct RST code block indentation to fix CI failure'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/investigate-ci-failure-JZ0IF

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[jpfeuffer]

Nope. Removing intensity cutoff is not an option.

[timosachsenberg]

why? size in memory? too slow? looks bad?
for some of them there are other solutions ;)

[github-actions]

[blacken-docs] _{reported by reviewdog 🐶}

pyopenms-docs/docs/source/user_guide/interactive_plots.rst

Lines 74 to 84 in 94824c8

	raster = (
	hd.rasterize(
	points,
	cmap=process_cmap("blues", provider="bokeh"),
	aggregator=ds.sum("inty"),
	cnorm="log",
	alpha=10,
	min_alpha=0,
	)
	.opts(active_tools=["box_zoom"], tools=["hover"], hooks=[new_bounds_hook])
	)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

docs/source/user_guide/interactive_plots.rst (2)

34-39: ThresholdMower usage looks good; consider briefly documenting the rationale

The filtering logic is fine and follows typical ThresholdMower usage (get defaults, set "threshold", apply in-place). Given prior questions in the PR about “why”, it might be worth adding a short comment or one sentence in the surrounding text explaining the motivation (e.g., reduce points for performance/memory, improve visual clarity of the plot).

---

43-45: Clarify the meaning of the new 1 argument to get2DPeakDataLong

The additional fifth argument may be correct for the updated API, but as a bare literal its intent isn’t obvious. Consider either:

• assigning it to a clearly named variable (e.g. ms_level = 1) and passing that, or
• adding a brief comment indicating what this parameter controls.

This will make the example more self-explanatory for readers not familiar with the full signature.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36b745e and 94824c8.

📒 Files selected for processing (1)

• docs/source/user_guide/interactive_plots.rst (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build-test

🔇 Additional comments (1)

docs/source/user_guide/interactive_plots.rst (1)

86-91: Indentation and .opts chaining on hd.dynspread look correct

Having hd.dynspread(...) indented inside the code block resolves the previous RST/notebook generation issue, and attaching width/height/xlabel/ylabel via .opts(...) on the final element is idiomatic holoviews usage. Nothing blocking here.

[jpfeuffer]

It can't be that this functionality was removed!

[jpfeuffer]

Loading is the bottleneck of this whole applet.

[timosachsenberg]

with BSA1.mzML ???
ok well, seems stupid DRange1 constructor is not supported or something like that. I would be happy if this would not be exposed in python at all.
Just two floats minIntensity maxIntensity