PR for v0.2.2 - #32
Merged
Merged
Conversation
- Added ylim argument to plotting functions - Enabled independent tick spacing for axes - Implemented grid plotting for 2D Bruker and DMFit data - Improved type safety and error handling - Refactored some code for clarity
Breaking Changes (API renames) - colors → color in bruker2d, bruker2d_grid, dmfit2d, dmfit2d_grid - proj_colors → proj_color in all 2D plot functions - homo → homonuclear in bruker2d, bruker2d_grid - diag → diagonal in all 2D plot functions - labels → titles in dmfit1d_grid (for subplot titles) Bug Fixes - read_nmr() now accepts tags="single_string" (no longer requires a list for one path) - All plot functions now warn on unrecognized kwargs instead of silently ignoring typos - Standardized save logic across all 9 plot functions (consistent defaults, no crashes) - Fixed __init__.py exports (all public functions now importable) - Standardized projection keys to "f1"/"f2" in Bruker 2D plots Internal Improvements - Extracted shared helpers into _helpers.py (save, tick spacing, nuclei labels, per-subplot limits) - dmfit1d now uses global DEFAULTS dict like all other functions - All plot functions accept spectrum dicts (unified input types) - Renamed tests/tests_io.py → test_io.py (was not being collected by pytest) Docs - Documented normalize parameters - Documented xlim/ylim list-of-tuples support in all grid functions - Updated user guide notebooks with new parameter names
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements version 0.2.2 of SpinPlots, addressing issues #29 and #31 with significant enhancements to plotting functionality and API consistency.
Changes:
- Added grid layouts for all plot types (
bruker2d_grid,dmfit1d_grid,dmfit2d_grid) - Unified parameter naming across all plot functions for consistency
- Added stacked plot support for 1D DMFit spectra and warning system for unrecognized kwargs
- Fixed
read_nmr()to accept single string tags - Updated documentation, README, CHANGELOG, and added CITATION.cff
Reviewed changes
Copilot reviewed 14 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_utils.py | Fixed regex pattern by escaping special characters |
| tests/test_spin.py | Updated tests for new grid functionality and SpinCollection changes |
| tests/test_plots.py | Added tests for new dmfit grid functions and kwarg validation |
| tests/test_io.py | Added tests for single-string tag support |
| src/spinplots/spin.py | Added SpinCollection convenience properties and updated plotting logic |
| src/spinplots/plot.py | Major refactoring: added grid functions, unified parameters, extracted helpers |
| src/spinplots/io.py | Fixed tag handling to accept single strings |
| src/spinplots/_helpers.py | New module with shared helper functions |
| src/spinplots/init.py | Updated version and exports |
| pyproject.toml | Version bump to 0.2.2 |
| docs/user_guide/read_export.ipynb | Fixed typo (31C → 13C) and updated examples |
| README.md | Improved quick start guide and feature list |
| CITATION.cff | Added comprehensive citation metadata |
| CHANGELOG.md | Detailed changelog for v0.2.2 |
| .gitignore | Added .claude/ directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 23 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| n_lines = sum(col.startswith("Line#") for col in dmfit_df.columns) | ||
|
|
||
| c = color[i] if isinstance(color, list) and i < len(color) else color |
There was a problem hiding this comment.
In dmfit1d, if color is a list shorter than the number of spectra, the else color branch will pass the entire list as a Matplotlib color spec (c = ... else color), which will raise at plot time. Consider using modulo indexing (e.g., color[i % len(color)]) or falling back to a default color when i >= len(color).
Suggested change
| c = color[i] if isinstance(color, list) and i < len(color) else color | |
| if isinstance(color, list): | |
| # If a list of colors is provided, cycle through them for each spectrum. | |
| # An empty list falls back to Matplotlib's default color cycle. | |
| c = color[i % len(color)] if len(color) > 0 else None | |
| else: | |
| c = color |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here are some of the things fixes in this PR, in line with #29 and #31
bruker2d_grid,dmfit1d_grid,dmfit2d_gridwere mising)