-
Notifications
You must be signed in to change notification settings - Fork 287
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
Add summary of ugrid to cube print out #3688
Conversation
338c0d2
to
5faf2b0
Compare
5faf2b0
to
6465a70
Compare
6465a70
to
5664c6b
Compare
ac1ba5d
to
bbecf2f
Compare
" " | ||
): | ||
node_var = self.dataset.variables[node_var_name] | ||
node_coordinates.append(node_var.getncattr("standard_name")) |
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.
I can't help feeling we should be applying the same here as in Iris --> standard_name or long_name or var_name.
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.
Yeah, I was a bit unsure about this bit. I'll do the different naming as you suggest.
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.
Looks OK, works fine on a number of files I tried, thanks @lbdreyer! Please could you address / push back on a few code layout things I've highlighted?
Just taking a quick look. |
Otherwise, only skimmed but LGTM : the additions to ugrid_cf_reader seem fine. |
Thanks @trexfeathers and @pp-mo for the review! I have dealt with your comments in the most recent commit. |
TBH this code area is a bit nasty, and I really don't envy you doing this, so 🥇 for taking it on @lbdreyer !! |
* Factor out cube.summary dim-naming + remove code for (nonexistent) multi-dim dim-coords. * Import cube.summary rework from #3688, but omitting ucube-specific parts. * Rename 'vector_summary' result variable of same-named function. * Use generic DimensionalMetadata.cube_dims in place of 'dim_function' arg. * Replace boilerplate calls to 'vector_summary' with generic code controlled by a table. * Define cube summary 'vector sections' via a function, which iris-ugrid can override. * Vector section summary titles used by cube.repr_html() are drawn from the cube itself. * Test for Cube._summary_dim_name(). * Tests for Cube._summary_vector_sections_info(). * Tiny docstring fixes. * Review changes. * Simpler calculation of vector elements, and handling of scalar elements. * Test fix.
DO NOT MERGE: Blocked by #3684NOW READY FOR REVIEW
This extends the cube repr to include ugrid information.
*
.ugrid information
) is added.The repr looks like:
And the repr htm looks like

Important things to note:
vertices per face: min 4 max 4
. I decided against this as to compute it, we need to load the data from the face_node_connectivity variable (if it exists) and considering this may be big I thought it wasn't worth the performance cost