- Notifications
You must be signed in to change notification settings - Fork 5
[FEAT] Add freshwater transport from Zheng et al. (2024), Issue #88 #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Added a new reader, metadata file and adjusted readers and standardise and the demo notebook.
| weblink: "https://zenodo.org/records/12790901" | ||
| comment: "Dataset accessed and processed via http://github.com/AMOCcommunity/amocatlas" | ||
| acknowledgement: > | ||
| "This study was supported by the National Natural Science Foundation of China (Grant 42122046, 42076202, 42075036), National Key Scientific and Technological Infrastructure project: Earth System Science Numerical Simulator Facility, (EarthLab) and the new Cornerstone Science Foundation through the XPLORER PRIZE. F. Li acknowledges the financial support from the National Key R&D Program of China (Grant 2023YFF0805102). Gratitude is extended to Elaine L. McDonagh, who graciously shared RAPID freshwater transport data. and the new Cornerstone Science Foundation through XPLORER PRIZE. F. Li acknowledges th financial support from the National Key R&D Program of China (Grant 2023YFF0805102). Gratitude is extended to Elaine L. McDonagh, who graciously shared RAPID freshwater transprot data." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate "and the new Cornerstone Science Foundation through XPLORER PRIZE. F. Li acknowledges th financial support from the National Key R&D Program of China (Grant 2023YFF0805102). Gratitude is extended to Elaine L. McDonagh, who graciously shared RAPID freshwater transprot data." also "transprot" --> "transport"
| Conventions: "CF-1.8, ACDD-1.3" | ||
| time_coverage_start: '2004-04-30' | ||
| time_coverage_end: '2020-12-31' | ||
| platform_type: "Salinity observations used in this study are from the Institute of Atmospheric Physics (IAP). Argo floats, CTD salinity sensors, bottles, mooring, sourced from the World Ocean Database (WOD). Precipitation and evaporation observations are derived from the Global Precipitation Climatology Project (GPCP)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use "platform_type"? I think for IOOS, this is supposed to be a limited set of options like "mooring array", not free text. Let's delete this one.
| platform_type: "Salinity observations used in this study are from the Institute of Atmospheric Physics (IAP). Argo floats, CTD salinity sensors, bottles, mooring, sourced from the World Ocean Database (WOD). Precipitation and evaporation observations are derived from the Global Precipitation Climatology Project (GPCP)." | ||
| contributor_name: "Huayi Zheng, Lijing Cheng, Feili Li, Yuying Pan, Chenyu Zhu" | ||
| contributor_role: "" | ||
| contributor_id: "https://orcid.org/0009-0004-5333-7595, https://orcid.org/0000-0002-9854-0392, https://scholar.google.com/citations?user=fXejLAwAAAAJ, https://orcid.org/0000-0001-7694-2625, https://orcid.org/0000-0002-9330-4294" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feili's ORCID is: https://orcid.org/0000-0002-3073-9813
| program: "amft" | ||
| description: "An observation based estimate of the Atlantic meridional freshwater transport" | ||
| project: "An observation based estimate of the Atlantic meridional freshwater transport" | ||
| weblink: "https://zenodo.org/records/12790901" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add "doi: "https://doi.org/10.5281/zenodo.12790900"" for the DOI of the dataset.
| AMFT_TRANSPORT_FILES = ["atl_mft_2000_extend_gpcp_oaflux.nc"] | ||
| AMFT_DEFAULT_SOURCE = "https://zenodo.org/records/12790901/files/" | ||
| | ||
| AMFT_METADATA = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to remember how we used the metadata in the read_<project>.py. I don't remember whether this overwrites what is in the _array.yml or not. In any case, can you delete the "acknowledgement" here because something funny happened with the spacing/formatting, and it'll pull that from the *.yml anyway. I'd also remove the doi and paper here. The project, weblink and comment can stay.
| time_coverage_start: '2004-04-30' | ||
| time_coverage_end: '2020-12-31' | ||
| platform_type: "Salinity observations used in this study are from the Institute of Atmospheric Physics (IAP). Argo floats, CTD salinity sensors, bottles, mooring, sourced from the World Ocean Database (WOD). Precipitation and evaporation observations are derived from the Global Precipitation Climatology Project (GPCP)." | ||
| contributor_name: "Huayi Zheng, Lijing Cheng, Feili Li, Yuying Pan, Chenyu Zhu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, for the naming overall, can you call it "read_Zheng2024" instead of read_amft? I think for the projects that don't have a recognisable name like "RAPID" or "OSNAP", we should name according to the paper/author/source. I guess 41N is a slight exception since it's named by latitude, but we can't do that here or for Calafat2025.
eleanorfrajka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - I suggested a few changes in the metadata. The demo notebook looks good.
Added a new reader, metadata file and adjusted readers and standardise and the demo notebook.
Description:
Added a new reader for the AMFT from Zheng et al. (2024)
Added new files:
read_amft.pyamft_array.ymlAdjusted the following files:
readers.py(to implement the new reader)standardise.py(to implement new data)Link any related issues, pull requests, or discussions:
Thoughts/ Problems:
I had to comment out a line in the init.py (
from ._version import __version__) or else i would get an error message when running the notebook:ModuleNotFoundError: No module named 'amocatlas._version'. I didn't run into that problem with the previous reader I added. When commenting the line everything worked and no errors were coming up.In the
demo.ipynbi did the subselecting of the data based on the latitude index in the plotting cell. I can still implement this into theplot_amoc_timeseriesfunction inplotters.py, but i thought it may be useful for users to not have to dig in the plotting function to see how to subselect data, but easily see it at first glance in the demonstration cell.