👌 Add scale_factors argument to plot_sas and 2 additional data color pairs#394
👌 Add scale_factors argument to plot_sas and 2 additional data color pairs#394s-weigand merged 17 commits intoglotaran:mainfrom
scale_factors argument to plot_sas and 2 additional data color pairs#394Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds documentation (dark-mode CSS, notebook), bumps dev/config files, exports a new cycler inspection utility with image previews, adds per-species SAS scaling and new plotting colors, and minor config/shell tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Cycler as Cycler object
participant Renderer as Matplotlib Renderer
participant Encoder as Base64 Encoder
participant Viewer as IPython/Markdown Viewer
User->>Cycler: provide cycler styles
Cycler->>Renderer: render tiny line plot per style (off-screen)
Renderer->>Encoder: return image bytes (JPEG)
Encoder->>Viewer: embed base64 image in HTML <img>
Viewer-->>User: display markdown table with style previews
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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. Comment |
Greptile OverviewGreptile SummaryThis PR adds per-species scaling functionality to Key Changes:
Documentation Quality: Confidence Score: 5/5
Important Files Changed
Flowchartflowchart TD
A[User calls plot_sas] --> B{scale_factors provided?}
B -->|Yes| C[Iterate through species]
B -->|No| C
C --> D{species in scale_factors?}
D -->|Yes| E[Apply scale_factor to data]
D -->|No| F[Use original data]
E --> G[Plot scaled species]
F --> G
H[User calls inspect_cycler] --> I[create_preview_cycler]
I --> J[Create matplotlib figure]
J --> K[Loop through cycler entries]
K --> L[Plot line with style]
L --> M[Save to BytesIO buffer]
M --> N[Encode as base64]
N --> O[Add to preview list]
O --> P{More entries?}
P -->|Yes| Q[Clear buffer with truncate]
Q --> K
P -->|No| R[Close figure]
R --> S[Return cycler with previews]
S --> T[Format as HTML table]
Last reviewed commit: 44ddd88 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #394 +/- ##
==========================================
- Coverage 60.70% 60.19% -0.52%
==========================================
Files 29 30 +1
Lines 1532 1570 +38
Branches 212 215 +3
==========================================
+ Hits 930 945 +15
- Misses 582 605 +23
Partials 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Line 42: The pre-commit configuration pins nbstripout to an unreleased commit
hash ("rev: 186d22f"); update the rev for the nbstripout entry in
.pre-commit-config.yaml to the latest tagged release (0.9.0) instead of the
commit hash so the hook uses the stable, reproducible tag; locate the nbstripout
entry by the package name "nbstripout" and replace the rev value accordingly.
In `@docs/notebooks/tips_and_tricks.ipynb`:
- Line 71: Fix the three minor typos in the notebook markdown cells by locating
and replacing the exact strings: change "From the fist look" to "From the first
look" (the cell containing "Result only contains a single dataset named
`dataset_1`"), change "Lucky for use" to "Lucky for us", and change "it's own
line" to "its own line"; update the corresponding markdown cells so the
corrected phrases replace the originals.
- Line 291: The variable name assigned from extract_irf_location is a typo
("if_location"); rename that variable to irf_location wherever it's assigned and
subsequently used (e.g., the statement currently "if_location =
extract_irf_location(ds)") so it matches the function and intended meaning;
update any downstream references in the same notebook cell or nearby cells to
use irf_location to avoid NameError and improve clarity.
In `@pyglotaran_extras/inspect/cycler.py`:
- Around line 38-55: The BytesIO buffer is reused in the loop and not truncated,
causing leftover bytes to corrupt subsequent JPEGs; in the loop around
fig.savefig(buffer, format="jpg") (inside the cycler iteration that builds
preview_images), ensure you reset and truncate the buffer between iterations by
seeking to 0 before savefig, then after savefig seek to 0, read the image bytes
into a local variable for the base64 encode, and then call buffer.truncate(0)
and buffer.seek(0) before the next iteration (references: buffer = io.BytesIO(),
fig.savefig(...), preview_images.append(...), ax.cla()).
🧹 Nitpick comments (1)
pyglotaran_extras/inspect/cycler.py (1)
40-41: Consider settingyto non-zero values for better line visibility in previews.With
y = [0] * 10, the line is drawn at the very bottom of the axes, which may make some line styles (especially dotted) hard to see. A midpoint value like[1] * 10with a fixed y-range might produce clearer previews.
s-weigand
force-pushed
the
new_options
branch
from
91f3360 to
fce6398
Compare
plot sas options to select species, and scale sas linetype argument added with plot concentrations
The lintype can easily be changed by using the cycler argument e.g.: cycler=PlotStyle().cycler * cycler(linestyle=['..'])
This can be done by selection the species before passing the data to the plotfunction res.sel(species=["species-1", "species-2"])
s-weigand
force-pushed
the
new_options
branch
from
7e6fb38 to
06eb475
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/notebooks/tips_and_tricks.ipynb`:
- Around line 192-193: Update the markdown text to fix grammar: replace the
sentence fragment "Even so we now have a line plot this isn't what we wanted
because each point on the `spectral` \n" with "Even though we now have a line
plot this isn't what we wanted because each point on the `spectral` \n" (locate
the notebook cell containing the "Even so we now have" phrase) and change the
phrase "For data with up to two dimension" to "For data with up to two
dimensions" (locate the cell containing that exact phrase).
In `@pyproject.toml`:
- Around line 101-121: The TOML under [tool.coverage] replaces coverage.py
defaults by setting report.exclude_lines, so add "pragma: no cover" back into
report.exclude_lines or better switch to using the newer exclude_also option
(coverage >=7.13.2) which extends defaults instead of replacing them; update the
config to remove the dead comment strings (the former .coveragerc lines
currently in report.exclude_lines) and either add "pragma: no cover" to
report.exclude_lines or change to exclude_also to preserve defaults while
keeping any custom patterns you need (refer to report.exclude_lines and
exclude_also in the [tool.coverage] section and remove the TOML-embedded comment
entries).
There was a problem hiding this comment.
I think the bots caught most of it, I don't see any more issues, just found a small issues about "plot_sas now mutates the input dataset when a scale factor is applied".
There's some overlap in the added documentation with the pyglotaran getting started guide so maybe at some point we can think of consolidating and/or putting in one place, or cross reference both, but better too much documentation than too little!
There was a problem hiding this comment.
Pull request overview
This PR adds enhanced plotting capabilities to pyglotaran-extras by introducing a scale_factors argument for per-species scaling in SAS plots, expanding the color palette with two new color pairs (olive/olive4 and turquoise/turquoise4), and adding a utility function to inspect plot cyclers. Additionally, it includes a comprehensive documentation notebook that teaches users how to work with the underlying libraries (xarray, matplotlib cyclers) to customize plots beyond the built-in options.
Changes:
- Added
scale_factorsparameter toplot_sasfor per-species amplitude scaling - Added 4 new color definitions (olive, olive4, turquoise, turquoise4) to
DataColorCodeenum - Added
inspect_cyclerutility function with preview generation for visualizing plot style cyclers
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Refactored configuration structure using nested syntax for hatch and tool settings; reorganized URL definitions alphabetically |
| pyglotaran_extras/plotting/style.py | Added four new color codes (olive, olive4, turquoise, turquoise4) to DataColorCode enum |
| pyglotaran_extras/plotting/plot_spectra.py | Added scale_factors parameter to plot_sas function for per-species scaling; converted coords to numpy arrays using .to_numpy() |
| pyglotaran_extras/plotting/plot_concentrations.py | Fixed docstring to correctly state default cycler is PlotStyle().cycler |
| pyglotaran_extras/inspect/cycler.py | New module with inspect_cycler and create_preview_cycler functions for visualizing plot style cyclers |
| pyglotaran_extras/inspect/init.py | Exported inspect_cycler function |
| justfile | Added -NoProfile flag to PowerShell invocation for Windows compatibility |
| docs/notebooks/tips_and_tricks.ipynb | New comprehensive tutorial notebook covering xarray data selection, plotting, and cycler customization |
| docs/index.md | Added tips_and_tricks notebook to documentation navigation |
| docs/conf.py | Added tips_and_tricks to exclude lists and included xarray_dark.css |
| docs/_static/css/xarray_dark.css | New CSS file for dark mode styling of xarray output and tables |
| docs/_static/css/mermaid_dark.css | Fixed typos: 'dakr' → 'dark', 'arror' → 'arrow' |
| .pre-commit-config.yaml | Updated dependency versions: pyproject-fmt (v2.11.1 → v2.16.0), nbstripout (0.9.0 → 186d22f), ruff (v0.14.14 → v0.15.1) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jsnel
left a comment
jsnel
left a comment
There was a problem hiding this comment.
It's good to go from my end now!
This change adds the new functionality requested by @ism200 .
As discussed in the initial request, I removed the additional arguments, which can be easily achieved using data selection or changing the cycler.
The reasoning for the removal is that we should rather teach users to help themselves do basic operations than add more function arguments, which increases the cognitive load and causes option fatigue.
Thus, I added a new documentation section "Going beyond builtin" explaining the basics of data selection and how to work with cyclers.
Change summary
scale_factorsargument toplot_sasfor scaling controlinspect_cyclerutility functionLinks to the changed docs sections
Checklist
Summary by CodeRabbit
New Features
Documentation
Chores