[FEAT] New NAC reader#134
Merged
Merged
Conversation
This commit features a new reader for North Atlantic current (NAC) transport estimates ba M. Lankhorst (2025). The new reader is set up with `nac.py`, which loads the data, `nac.yml` which holds the metadata and variable mapping and `test_nac.py` which tests the new reader. Also, other files were changed to account for the new reader: `__init__.py`, `read.py`, `readers.py`, `standardise.py`. In addition to that the `demo.ipynb` was extended to load and show both transport estimates: `TRANS_NAC` which is derived from satellite and float observations and `TRANS_FBC_PROXY` which is derived just from satellite altimetry. Changes were also made in `plotters.py`. Now data products which have a coarser resolution than 1 month can be plotted in a desired color as well, when `resample_monthly` is set to `False`. The change in `utilities.py` accounts for an error which was given, when a loaded dataset has a `TIME` coordinate stored as `datetime64`, but also carries `valid_min` and/or `valid_max` metadata that is numeric. Therefore the masking code from function `utilities.mask_invalid_values()` now skips datetime variables.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for the Lankhorst (2025) North Atlantic Current (NAC) transport time series as a new AMOCatlas datasource, including YAML-driven standardization metadata and integration into the public read/load APIs. Also includes a small plotting behavior tweak and a utilities fix to avoid masking errors on datetime variables with numeric valid_min/valid_max attrs.
Changes:
- Introduce a new NAC reader (
amocatlas/data_sources/nac.py) plus YAML metadata/variable mapping (amocatlas/metadata/nac.yml) and basic tests. - Wire the new reader into the package entrypoints (
data_sources/__init__.py,readers.py,read.py) and extend the demo notebook. - Update plotting behavior for raw series coloring when
resample_monthly=False, and skip datetime variables inmask_invalid_values().
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_nac.py | Adds basic structural tests for the new NAC reader module. |
| notebooks/demo.ipynb | Demonstrates loading/plotting NAC transport and proxy variables. |
| amocatlas/utilities.py | Skips datetime variables during valid-range masking to avoid type errors. |
| amocatlas/standardise.py | Adds deprecated standardise_nac() wrapper for compatibility. |
| amocatlas/readers.py | Registers nac reader in the internal reader dispatch. |
| amocatlas/read.py | Exposes read.nac() and includes NAC in the public API and __all__. |
| amocatlas/plotters.py | Adjusts raw plotting color/alpha/linewidth behavior when not resampling monthly. |
| amocatlas/metadata/nac.yml | Adds NAC global + per-file metadata and variable mapping for standardization. |
| amocatlas/data_sources/nac.py | Implements the NAC reader and associated constants/metadata. |
| amocatlas/data_sources/init.py | Re-exports read_nac at the amocatlas.data_sources package level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Minor changes to `standardise.py`, `plotters.py` regarding formatting issues. Adjustments in `nac.py` to account for new data file name, which results from the url, when loading the dataset off of the website. Changed the `test_nac.py` accrodingly. Implemented an extension of the test in `test_utilities.py`, called `test_mask_invalid_values()`, to account for the changes commited earlier regarding datetime64 objects.
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.
This commit features a new reader for North Atlantic current (NAC) transport estimates by M. Lankhorst (2025).
Description:
The new reader is set up with
nac.py, which loads the data,nac.ymlwhich holds the metadata and variable mapping andtest_nac.pywhich tests the new reader.Also, other files were changed to account for the new reader:
__init__.py,read.py,readers.py,standardise.py.In addition to that, the
demo.ipynbwas extended to load and show both transport estimates:TRANS_NACwhich is derived from satellite and float observations andTRANS_NAC_PROXYwhich is derived just from satellite altimetry.Changes were also made in
plotters.py. Now data products which have a coarser resolution than 1 month can be plotted in a desired color as well, whenresample_monthlyis set toFalse.The change in
utilities.pyaccounts for an error which was given, when a loaded dataset has aTIMEcoordinate stored asdatetime64, but also carriesvalid_minand/orvalid_maxmetadata that is numeric. Therefore the masking code from functionutilities.mask_invalid_values()now skips datetime variables.Related Issues / Pull Requests:
Link any related issues, pull requests, or discussions: