-
Notifications
You must be signed in to change notification settings - Fork 300
Support name tokens #3399
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
Support name tokens #3399
Conversation
|
@lbdreyer I still need to add some test coverage to the PR... just pushed this so that you can see the approach I'm taking 😉 |
| import six | ||
|
|
||
| # TODO: Is this a mixin or a base class? | ||
|
|
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.
Horse. Bolted.
|
@lbdreyer Okay, I think that I'm pretty much there... As it happens, there wasn't any unit test coverage of Being a good citizen, I'll add test coverage in a separate PR. I've also not updated the docs either, just so that this PR can get banked sooner rather than later, but happy to make a follow-up PR, if you think that's appropriate... 😄 |
lbdreyer
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.
Nice addition and great to see much more test coverage!
|
@lbdreyer I've actioned all your review comments, |
|
All good 👍 👍 Thanks @bjlittle ! |
This PR is the outcome of a side discussion regarding issue #3360.
In short, in
iriswe've allowed the use oflong_namefrom a coordinate to be used in the construction of theCellMethod, but that causes issues when thelong_namecontains one or more space (" ") characters, as it then quickly becomes ambiguous to parse the grammar suggested in the CF Conventions for acell_methodsattribute on loading after saving theCellMethodto NetCDF. That's not a fault of the CF Conventions, more our interpretation and implementation of it (from a file format agnostic point of view).Reference, Cell Methods, 2nd paragraph:
That said, this PR attempts to resolve this issue by ensuring that the
coord_namesandmethodused within the construction of aCellMethodare valid NetCDF name tokens, and thus can be parsed within the rules suggested in the CF Conventions without ambiguity (or over-engineering 😉).