-
Notifications
You must be signed in to change notification settings - Fork 283
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
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 😉 |
@@ -19,15 +19,18 @@ | |||
from six.moves import (filter, input, map, range, zip) # noqa | |||
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... 😄 |
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
iris
we've allowed the use oflong_name
from a coordinate to be used in the construction of theCellMethod
, but that causes issues when thelong_name
contains one or more space (" ") characters, as it then quickly becomes ambiguous to parse the grammar suggested in the CF Conventions for acell_methods
attribute on loading after saving theCellMethod
to 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_names
andmethod
used within the construction of aCellMethod
are valid NetCDF name tokens, and thus can be parsed within the rules suggested in the CF Conventions without ambiguity (or over-engineering 😉).