Grid based interface fixes

[philippjfr]

Ensure that the flat and non-flat arrays returned by the grid interfaces are consistent and of the correct orientation. Also ensures the constructors work correctly and all process tuples of the coordinates and arrays correctly. Also adds small fixes for the GridImage Element, updates test and fixes issues with dynamic groupby, which caused the grid interface to reject the data in some cases. Addresses #789 and more.

Still want to add a number of tests to ensure consistency and stability of the interfaces.

• Standardized interface to get canonical views of coordinates and data arrays
• Fixed constructors ensuring they handle standard tuple and dict based gridded data correctly
• Fixed GridImage density calculation
• Fixes for dynamic groupby with gridded interfaces (and allowing passing of parameters to groups).
• Added unit tests to check output of gridded data dimension_values is consistent.

[jlstevens]

As long as you are sure this is definitely correct now... :-)

[jlstevens]

It is probably too much of a pain to stick in the docstring but it would be good to describe our canonical format properly somewhere in our docs. Not something that should hold up this PR though!

[jlstevens]

Looks good!

I've made most of my key comments now and in general I am happy with this. I didn't look in detail at the changes in the interface methods that already exist - I assume those are fixes as opposed to API changes.

[philippjfr]

Thanks for the review.

I assume those are fixes as opposed to API changes.

Yes mostly changes to correctly handle constant dimensions, actually integrating constant dimensions properly might actually be helpful here in future.

[jlstevens]

actually integrating constant dimensions properly might actually be helpful here in future.

Does xarray, iris etc have a corresponding concept? As I think they might, in a separate PR, we might want to consider a minimal API for setting/accessing constant dimensions on interfaces.

It is a little tricky in some cases e.g wrapping numpy arrays + bounds as in such a case, I'm not sure where this state would actually live.

[philippjfr]

Does xarray, iris etc have a corresponding concept? As I think they might, in a separate PR, we might want to consider a minimal API for setting/accessing constant dimensions on interfaces.

Iris does have explicit support and xarray at least highlights which dimensions are constant in the repr. The special handling is mostly about removing the constant axes in the array, e.g. reducing an (100, 50, 1, 1) shaped array to (100, 50). But it would be nice not to throw those constant coordinates away and turn them into constant dimensions instead.

[philippjfr]

I have now applied all your comments except for folding in expanded_coords. As it is now it can be implemented once on the GridInterface baseclass, if I fold it into the coords method I'll be duplicating it for each of the coords implementations.

[jlstevens]

if I fold it into the coords method I'll be duplicating it for each of the coords implementations.

Would that be true if you made expanded_coords into a utility?

I can imagine each coords implementation having the signature:

def coords(cls, dataset, dim, ordered=False, expanded=False):

and then all ending with something like:

return util.expanded_coords(data, expanded)

[philippjfr]

Okay done that as well now.

[jlstevens]

Looks good! I'll merge as soon as the tests pass.

[jlstevens]

Tests passed. Merging.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.