Skip to content

Trying to sync to more official version of tables - #124

Merged
maritsandstad merged 10 commits into
mainfrom
tables-update
Oct 7, 2026
Merged

maritsandstad merged 10 commits into
mainfrom
tables-update

Conversation

@maritsandstad

Copy link
Copy Markdown
Collaborator

This is a first step towards getting official versions of tables and grids into our flow.

@maritsandstad

Copy link
Copy Markdown
Collaborator Author

@AgentOxygen and @mvertens this is very much work in progress, and needs more help with scientific testing eventually, but I do think it is useful to make this PR so that we can discuss the direction.

I do also think your new scheme in #119 can be related to this as this was actually quite fiddly todo, partially because it was often far from clear where we were pulling data for to run the flow, and definitely we were not doing it in the best ways.

@maritsandstad
maritsandstad marked this pull request as ready for review October 5, 2026 20:37
@maritsandstad
maritsandstad requested a review from mvertens October 5, 2026 20:37
@mvertens

mvertens commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@maritsandstad - the following issues were raised by Claude:

  • The new code isn't tested. 191 lines decide what goes into every submitted file, and nothing checks them directly. It's a function that takes arguments and returns a dictionary, so testing it is easy — worth doing before this becomes the only path.
  • Some values quietly fall back to defaults. If a lookup fails, the code picks something and carries on: grid_label becomes "gr", activity_id becomes "CMIP", and so on. Those land in the files. The grid label worries me most, because CMIP7 uses codes like g100 and "gr" may not be valid at all — and a warning buried in a long log won't be noticed.
  • Two attributes could come out empty. data_specs_version and Conventions are read straight from the CV with no fallback, so if the new tables don't have them at the top level you get nulls in the output.

@mvertens mvertens added the enhancement New feature or request label Oct 6, 2026
@maritsandstad

Copy link
Copy Markdown
Collaborator Author

@maritsandstad - the following issues were raised by Claude:

  • The new code isn't tested. 191 lines decide what goes into every submitted file, and nothing checks them directly. It's a function that takes arguments and returns a dictionary, so testing it is easy — worth doing before this becomes the only path.
  • Some values quietly fall back to defaults. If a lookup fails, the code picks something and carries on: grid_label becomes "gr", activity_id becomes "CMIP", and so on. Those land in the files. The grid label worries me most, because CMIP7 uses codes like g100 and "gr" may not be valid at all — and a warning buried in a long log won't be noticed.
  • Two attributes could come out empty. data_specs_version and Conventions are read straight from the CV with no fallback, so if the new tables don't have them at the top level you get nulls in the output.

So to Claude, we could have testing for it, but it is not super meaningful testing in my opinion, and what we had before was definitely just a file that hung out...

For the second, that is absolutely on purpose so we can have something functioning also for non-CMIP runs.

The third for now is also a typical Claude wanting belt and suspenders for everything thing. The CV should work, if it doesn't there is no reason to have a fall-back...

@maritsandstad
maritsandstad merged commit 2dad681 into main Oct 7, 2026
2 checks passed
@maritsandstad
maritsandstad deleted the tables-update branch October 7, 2026 07:50
@maritsandstad maritsandstad linked an issue Oct 7, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update the cmor-tables version

2 participants