-
Notifications
You must be signed in to change notification settings - Fork 10
New variables for CTD_BGC #201
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
Conversation
ammedd
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.
Looks fine to me!
tests/instruments/test_ctd_bgc.py
Outdated
| "no3": no3, | ||
| "po4": po4, | ||
| "ph": ph, | ||
| "phytoplankton": phytoplankton, |
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.
Why use the full names here and not the abbreviations?
"phyc",
"zooc",
"nppv",
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.
Now using abbreviations consistently 👍
erikvansebille
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.
In general, I would stick as close to the Copernicus Marine terminology for these fields as possible; so no3 instead of nitrate etc?
Yes, I've updated this now in my latest commit, as also suggested by @ammedd 👍 |
| "nitratedata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-nut_anfc_0.25deg_P1D-m", | ||
| "variables": ["no3"], | ||
| "output_filename": "ctd_bgc_nitrate.nc", | ||
| }, | ||
| "phosphatedata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-nut_anfc_0.25deg_P1D-m", | ||
| "variables": ["po4"], | ||
| "output_filename": "ctd_bgc_phosphate.nc", | ||
| }, | ||
| "phdata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-car_anfc_0.25deg_P1D-m", | ||
| "variables": ["ph"], | ||
| "output_filename": "ctd_bgc_ph.nc", | ||
| }, | ||
| "phytoplanktondata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-pft_anfc_0.25deg_P1D-m", | ||
| "variables": ["phyc"], | ||
| "output_filename": "ctd_bgc_phytoplankton.nc", | ||
| }, | ||
| "zooplanktondata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-plankton_anfc_0.25deg_P1D-m", | ||
| "variables": ["zooc"], | ||
| "output_filename": "ctd_bgc_zooplankton.nc", | ||
| }, | ||
| "primaryproductiondata": { | ||
| "dataset_id": "cmems_mod_glo_bgc-bio_anfc_0.25deg_P1D-m", | ||
| "variables": ["nppv"], | ||
| "output_filename": "ctd_bgc_primary_production.nc", | ||
| }, |
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.
There are still a few inconsistencies between the naming in Copernicus Marine data and the naming in VirtualShip here?
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.
Oh yes! Now fixed as well
To get VirtualShip ready for the requirements of upcoming university courses, the
CTD_BGCinstrument now supports measurements of additional variables: nutrients (nitrate, phosphate), pH, phytoplankton, zooplankton and primary production (on top of the pre-existing o2 and chlorophyll variables).Tests have also been updated to accommodate the new variables.