Skip to content

Cube.coords mesh_coords key [AVD-1662]#4063

Merged
bjlittle merged 2 commits intoSciTools:mesh-data-modelfrom
pp-mo:cube_coords_mesh
Mar 15, 2021
Merged

Cube.coords mesh_coords key [AVD-1662]#4063
bjlittle merged 2 commits intoSciTools:mesh-data-modelfrom
pp-mo:cube_coords_mesh

Conversation

@pp-mo
Copy link
Copy Markdown
Member

@pp-mo pp-mo commented Mar 12, 2021

🚀 Pull Request

Description

Adding a 'mesh_coords' keyword to Cube.coords.
Behaves mostly like the existing dim_coords keyword.

(This is a simple preliminary to adding the more complex cube api to come :

  • cube.mesh and .location
  • cube.__eq__ should compare mesh and location
  • various checks that any MeshCoord-s in a cube are always compatible.
    )

Consult Iris pull request check list

@pp-mo pp-mo force-pushed the cube_coords_mesh branch from 8ba2662 to 68d1b9c Compare March 12, 2021 19:36
# But put these *afterward*, unlike other similar classes.
for item in ("standard_name", "units", "long_name", "attributes"):
for item in (
"shape",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pp-mo Snuck in shape here... no objections, just curious as to the reasoning

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it seems clear that when printing these mesh information components, we don't want to realise or print out contents.
But when debugging I saw the printouts, it seemed odd you couldn't even see the shape., unlike dask arrays which show the shape + dtype.
So I added it in (but not dtype).

Since then I realised that you + @trexfeathers had already agreed a printout format for Connectivity, which does not include shape.
So I now think that we should probably at least make those 2 the same :
What do you think + which way do you reckon we should we jump ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pp-mo Awesome, thanks.

I was going to re-haul how coordinates in general do __str__ and __repr__ in order to keep everything lazy.

I think there is real tangible benefit there, but outside the scope of this PR

Copy link
Copy Markdown
Member

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pp-mo Lovely how easily this integrates into iris 😄

Awesome, thanks! 👍

Just a super minor question to answer (which isn't a block to this PR merging)

@bjlittle bjlittle merged commit de2372b into SciTools:mesh-data-model Mar 15, 2021
@pp-mo pp-mo deleted the cube_coords_mesh branch March 18, 2022 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants