Skip to content

Improve rssa cyl plots#190

Merged
dodu94 merged 3 commits intodevelopingfrom
improve-rssa-cyl-plots
Feb 5, 2026
Merged

Improve rssa cyl plots#190
dodu94 merged 3 commits intodevelopingfrom
improve-rssa-cyl-plots

Conversation

@AlvaroCubi
Copy link
Collaborator

@AlvaroCubi AlvaroCubi commented Feb 5, 2026

The cylindrical plotting of RSSA files used to show the theta lower values at the left of the plot, this is akin as looking at a cylinder from the outside. We are used to viusalizations where we study cylinders from the inside, this change makes confusions less likely.

Summary by CodeRabbit

  • Breaking Changes

    • Removed a public API method for retrieving energy spectra from the RSSA interface.
  • Bug Fixes

    • Fixed perimeter position/orientation in radial plotting so rendered cylindrical views appear correctly (angle flipped to simulate inside-facing perspective).

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

Walkthrough

Removed an unused public method get_energy_spectra and its Sequence import from the RSSA class; adjusted the theta sign in perimeter position calculation in the plotting module (perimeter positions now use a flipped theta).

Changes

Cohort / File(s) Summary
RSSA API Cleanup
src/f4enix/output/rssa/rssa.py
Removed public method RSSA.get_energy_spectra(self, energy_bins: Sequence[float]) -> pl.DataFrame and removed the Sequence import from collections.abc.
Perimeter Position Sign Adjustment
src/f4enix/output/rssa/rssa_plotting.py
Changed theta computation to use arctan2(y, -x), effectively flipping the sign used when computing perimeter_pos (now uses -theta * r semantics).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰✨ I nudged a method out the door,

Turned theta round and hopped some more.
RSSA hums, positions spin,
A tiny change, a quieter din.
🥕🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Improve rssa cyl plots' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific change (flipping theta orientation in cylindrical plotting). Consider a more specific title that describes the actual change, such as 'Flip theta orientation in RSSA cylindrical plots' or 'Display lower theta values on right in RSSA cylinder visualization'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-rssa-cyl-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.

Copy link
Member

@dodu94 dodu94 left a comment

Choose a reason for hiding this comment

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

no comments

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/f4enix/output/rssa/rssa.py 100.00% <ø> (+1.56%) ⬆️
src/f4enix/output/rssa/rssa_plotting.py 93.69% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dodu94 dodu94 merged commit 1b921a2 into developing Feb 5, 2026
8 checks passed
@dodu94 dodu94 deleted the improve-rssa-cyl-plots branch February 5, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants