199 data request 20km regional projections for cordex cmip6 queensland future climate science #219

charles-turner-1 · 2024-10-14T04:15:36Z

Added Cordex CMIP6 Translators
Beginning refactoring @rbeucher noted would be useful in Add support for BARPA py18 data store #211. There's a little more work to do here so I've left this as a draft PR for now.

@marc-white You added config/experiments/cmip6_ig45.yaml in this branch - I've separately added a config/metadata_sources/cordex_ig45.yaml which is doing largely the same thing but seemed to follow the other source conventions more closely. Are you able to confirm whether there's a convention I'm unaware of?

Closes #199

* Replaced a couple of try/excepts with .get in `src/access_nri_intake/catalog/translators.py` * Updated a misleading docstring

…nate variable searching & indexing

… searching for coordinate variables

likely to be necessary for passing around coordinates as well as data variables as we begin to make coordinates indexable - I think we'll begin to get confused about what belongs where.

dataclasses that are fed into a dataclass holding all the coordinate and data variables from a netCDF file. - Updated tests so that they are all passing: tests now expect to find coordinate variables from the netCDF files as well as data variables. - Some minor changes to make code more readable - changed long tuples to dataclasses & dictionaries where possible.

…due to use of '|' type union syntax - might be worth consideration? - Removed a couple of unused imports, cleaned up some comments

…nal[T], etc)

… search (forgot to test)

- Updated tests to better respect moving coordinate variables back into variables - Moved mypy setup stuff into to 'pre-commit-config.yaml'

…gional-projections-for-cordex-cmip6-queensland-future-climate-science Most interested in keeping typing hanging around for development speed.

…portant, causing unnecessary test failures)

…NRI/access-nri-intake-catalog into 660-coordinate-variables

…ex.yaml and config/metadata_sources/cordex-ig45/metadata.yaml

…al-projections-for-cordex-cmip6-queensland-future-climate-science

…-20km-regional-projections-for-cordex-cmip6-queensland-future-climate-science" This reverts commit 27e460f, reversing changes made to 0c96e28.

codecov · 2024-10-14T04:17:56Z

Codecov Report

Attention: Patch coverage is 98.38710% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.92%. Comparing base (d66ff09) to head (35c20fb).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/access_nri_intake/catalog/translators.py	98.38%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #219      +/-   ##
==========================================
- Coverage   97.00%   96.92%   -0.09%     
==========================================
  Files           9        9              
  Lines         635      650      +15     
==========================================
+ Hits          616      630      +14     
- Misses         19       20       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

marc-white

@charles-turner-1 there isn't a convention for naming the metadata sources that we're storing in the repository. This is a very recent change that was made to avoid repeating issues like #200. However, they're only reference copies, and the catalog itself should reference the metadata.yaml stored somewhere on /g/data. Therefore, you can really do what you like with this, so long as we stay internally consistent.

marc-white · 2024-10-14T05:14:26Z

config/experiments/cmip6_ig45/metadata.yaml

+frequency:
+- 
+variable:
+- 
+nominal_resolution:
+- 


You can just leave these as blank-blank, rather than a blank array.

marc-white · 2024-10-14T05:14:33Z

config/experiments/cmip6_ig45/metadata.yaml

+related_experiments:
+-


marc-white · 2024-10-14T05:18:24Z

config/cmip6.yaml

+  - metadata_yaml: /g/data/xp65/admin/intake/metadata/cmip6_ig45/metadata.yaml
+    path:
+    - /g/data/ig45/catalog/v2/esm/catalog.json 


If this is the path where the 'live' metadata.yaml is being stored, then I'd recommend the reference copy's name/path within the repo match, i.e. be cmip6_ig45.

marc-white · 2024-10-14T05:24:13Z

config/cordex.yaml

+
+sources:
+
+  - metadata_yaml: /g/data/xp65/admin/access-nri-intake-catalog/config/metadata_sources/cordex-ig45/metadata.yaml


I don't think this is a path where we want to be storing live YAML... see discussion on #200

marc-white

As it currently stands, it looks like you're going to load the same Intake-ESM catalogue into access-nri-intake-catalog twice, using two different translators and metadata.yaml files. I presume you wanted to use the cordex ones in the final merge?

sentry) - Updated cmip6.yaml as a different translator is required for Cordex experiments as main CMIP6 experiments.

charles-turner-1 · 2024-10-14T08:25:34Z

Yup - the changes to cmip6.yaml seem to go with the draft metadata.yaml you added. The cordex experiments needed a different translator, so I've separated them out - just hadn't cleaned up that file - everything should be sensible now.

I'm not 100% sure I understand the discussion around YAML paths fully - either that or it appears we need to update a bunch of paths in config/*.yaml?

marc-white · 2024-10-14T23:23:50Z

@charles-turner-1 There's a few things about YAML paths:

Typically, the metadata.yaml lives with the data. However, there are some "historical" experiments for which we don't have write access to their data area, so we have to store the metadata.yaml within our own xp65 project. Where in xp65 was debated, given there is also an access-nri-intake-catalog checkout in there somewhere we don't want to fiddle with/be dependent on in production.
There were some experiments that we were already holding the metadata for in xp65, but it accidentally got deleted. That's why we're holding all the metadata for the live catalog in a single place (/g/data/xp65/admin/intake/metadata) to try and stop that happening again.
Separately, @rbeucher asked that we start holding a reference copy of new experiment metadata.yamls within the access-nri-intake-catalog repository. This isn't for production use; it's just for reference in case the above problem happens again (and I guess might be handy for testing too, now that I think about it).

…or-cordex-cmip6-queensland-future-climate-science

- Updated path for cordex.yaml file

charles-turner-1 · 2024-10-17T01:34:56Z

config/cordex.yaml

@marc-white Can you confirm this is where we are wanting to store our metadata files? Just wanna double check I've understood correctly.

Yes that is correct.

rbeucher

I think we need to move out of draft test to run the tests.

rbeucher · 2024-10-17T03:45:31Z

config/cmip6.yaml

@@ -10,4 +10,4 @@ sources:

  - metadata_yaml: /g/data/xp65/admin/access-nri-intake-catalog/config/metadata_sources/cmip6-oi10/metadata.yaml
    path:
-      - /g/data/oi10/catalog/v2/esm/catalog.json
+      - /g/data/oi10/catalog/v2/esm/catalog.json


Add empty line at the end of the file

rbeucher · 2024-10-17T03:45:57Z

config/cordex.yaml

Yes that is correct.

rbeucher · 2024-10-17T03:46:13Z

config/metadata_sources/cordex-ig45/metadata.yaml

+related_experiments:
+notes: 
+keywords:
+- cmip


Add empty line at the end of the file

rbeucher · 2024-10-17T03:51:41Z

tests/data/esm_datastore/cordex-ig45.json

+            "column_name": "time_range"
+        }
+    ]
+}


Add empty line at the end of the file

charles-turner-1 · 2024-10-17T04:20:11Z

@rbeucher my bad, moved it out of draft now.

charles-turner-1 and others added 26 commits September 12, 2024 10:05

Updated .gitignore to include some vs code settings

95e53e7

* Added type hints

13b38a9

* Replaced a couple of try/excepts with .get in `src/access_nri_intake/catalog/translators.py` * Updated a misleading docstring

More type hints

f89ece1

Type hint upddates

aeaa970

Updated data_vars => variables in parse_access_ncfile to allow coordi…

6eb659e

…nate variable searching & indexing

Added a bunch of type annotations & starting to look at how we fit in…

f36050f

… searching for coordinate variables

Updated all the builders to use a dataclass rather than a tuple:

ca1743e

likely to be necessary for passing around coordinates as well as data variables as we begin to make coordinates indexable - I think we'll begin to get confused about what belongs where.

- Added mypy checks for 3.9..3.12. Note type hints fail on Python3.9 …

7f34102

…due to use of '|' type union syntax - might be worth consideration? - Removed a couple of unused imports, cleaned up some comments

Rewrote type hints to be compatible with Python3.9 (T | None => Optio…

bc0265c

…nal[T], etc)

Fixed some issues tests weren't catching

f84d383

Renamed => - makes indexing more consistent

7799c8d

Fixed a couple of issues relating to changing from coords => coord in…

e6590c9

… search (forgot to test)

Add cmip6_ig45 to catalog

0c96e28

- Moved coordinate variables back into variables (simpler interface)

1d9f429

- Updated tests to better respect moving coordinate variables back into variables - Moved mypy setup stuff into to 'pre-commit-config.yaml'

Merge branch 'main' into 660-coordinate-variables

3e8ad02

Merge branch '660-coordinate-variables' into 199-data-request-20km-re…

27e460f

…gional-projections-for-cordex-cmip6-queensland-future-climate-science Most interested in keeping typing hanging around for development speed.

Cleaned up _cmip_realm_translator & added sorting to test (order unim…

396df8e

…portant, causing unnecessary test failures)

Merge branch 'main' into 660-coordinate-variables

40b055e

Merge branch '660-coordinate-variables' of https://github.com/ACCESS-…

73265b5

…NRI/access-nri-intake-catalog into 660-coordinate-variables

Added _DispatchKeys dataclass to hold tagnames

babc3da

Refactored a bunch of the translations out to the DefaultTranslator

0e2d669

Updated translators.py to include CordexTranslator, added config/cord…

28a8a46

…ex.yaml and config/metadata_sources/cordex-ig45/metadata.yaml

Working cordex translator

8dc5c2a

gerge branch 'refactor_translators' into 199-data-request-20km-region…

b3dc52d

…al-projections-for-cordex-cmip6-queensland-future-climate-science

Revert "Merge branch '660-coordinate-variables' into 199-data-request…

b3da577

…-20km-regional-projections-for-cordex-cmip6-queensland-future-climate-science" This reverts commit 27e460f, reversing changes made to 0c96e28.

charles-turner-1 linked an issue Oct 14, 2024 that may be closed by this pull request

[DATA REQUEST] 20km regional projections for CORDEX-CMIP6 from the Queensland Future Climate Science Program #199

Open

5 tasks

marc-white self-requested a review October 14, 2024 05:09

marc-white reviewed Oct 14, 2024

View reviewed changes

config/experiments/cmip6_ig45/metadata.yaml Outdated

Comment on lines 22 to 23

related_experiments:

-

Copy link

Collaborator

marc-white Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

marc-white reviewed Oct 14, 2024

View reviewed changes

- Updated tests (covered missing lines introduced by refactor found by

b303840

sentry) - Updated cmip6.yaml as a different translator is required for Cordex experiments as main CMIP6 experiments.

charles-turner-1 force-pushed the 199-data-request-20km-regional-projections-for-cordex-cmip6-queensland-future-climate-science branch from f9dbb95 to b303840 Compare October 14, 2024 08:17

charles-turner-1 changed the base branch from main to 186-return-type October 15, 2024 03:22

charles-turner-1 changed the base branch from 186-return-type to main October 15, 2024 03:22

charles-turner-1 added 2 commits October 16, 2024 14:02

gerge branch 'main' into 199-data-request-20km-regional-projections-f…

6deea5d

…or-cordex-cmip6-queensland-future-climate-science

- Renamed experiment in metadata.yaml

cd0e241

- Updated path for cordex.yaml file

charles-turner-1 commented Oct 17, 2024

View reviewed changes

charles-turner-1 requested a review from rbeucher October 17, 2024 03:16

rbeucher reviewed Oct 17, 2024

View reviewed changes

Add missing newlines to end of yaml & json files

35c20fb

charles-turner-1 marked this pull request as ready for review October 17, 2024 04:15

rbeucher approved these changes Oct 17, 2024

View reviewed changes

rbeucher merged commit 761e7f4 into main Oct 17, 2024
15 checks passed

charles-turner-1 deleted the 199-data-request-20km-regional-projections-for-cordex-cmip6-queensland-future-climate-science branch November 1, 2024 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

199 data request 20km regional projections for cordex cmip6 queensland future climate science #219

199 data request 20km regional projections for cordex cmip6 queensland future climate science #219

charles-turner-1 commented Oct 14, 2024 •

edited

Loading

codecov bot commented Oct 14, 2024 •

edited

Loading

marc-white left a comment

marc-white Oct 14, 2024

marc-white Oct 14, 2024

marc-white Oct 14, 2024

marc-white Oct 14, 2024

marc-white left a comment

charles-turner-1 commented Oct 14, 2024

marc-white commented Oct 14, 2024

charles-turner-1 Oct 17, 2024

rbeucher Oct 17, 2024

rbeucher left a comment

rbeucher Oct 17, 2024

rbeucher Oct 17, 2024

rbeucher Oct 17, 2024

rbeucher Oct 17, 2024

charles-turner-1 commented Oct 17, 2024


		sources:

		- metadata_yaml: /g/data/xp65/admin/access-nri-intake-catalog/config/metadata_sources/cordex-ig45/metadata.yaml

199 data request 20km regional projections for cordex cmip6 queensland future climate science #219

199 data request 20km regional projections for cordex cmip6 queensland future climate science #219

Conversation

charles-turner-1 commented Oct 14, 2024 • edited Loading

codecov bot commented Oct 14, 2024 • edited Loading

Codecov Report

marc-white left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-white left a comment

Choose a reason for hiding this comment

charles-turner-1 commented Oct 14, 2024

marc-white commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbeucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charles-turner-1 commented Oct 17, 2024

charles-turner-1 commented Oct 14, 2024 •

edited

Loading

codecov bot commented Oct 14, 2024 •

edited

Loading