-
Notifications
You must be signed in to change notification settings - Fork 296
Make MeshCoords immutable and sync updates with the attached mesh #6405
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
base: main
Are you sure you want to change the base?
Conversation
aux_coords_and_dims=[(meshco_1, 0), (meshco_2, 0)], | ||
) | ||
assert cube.mesh == meshco_1.mesh | ||
# def test_multi_meshcoords_same_axis(self): |
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.
@ESadek-MO agreed we can lose this test, as this awkward possibility no longer exists.
Since meshcoord metadata is now taken from the mesh itself, I think we can't now get 2 different meshcoords on the same axis
new_meshco_y.rename("alternative") | ||
cube.add_aux_coord(new_meshco_y, 1) | ||
assert len(cube.coords(mesh_coords=True)) == 3 | ||
# def test_add_multiple(self): |
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.
As above, think this possibility is now obsolete + we can ditch this test.
# # NOTE: the meshcoord.axis takes precedence over the older | ||
# # "guessed axis" approach. So the standard_name does not control it. | ||
# coord_on_mesh = meshcoord.mesh.coord( | ||
# location=meshcoord.location, | ||
# axis=meshcoord.axis | ||
# ) | ||
# #doesn't like renaming to latitude....... | ||
# #any other one works | ||
# coord_on_mesh.standard_name = "grid_longitude" | ||
# self.assertIs(cube.coord(axis="x"), meshcoord) | ||
# self.assertEqual(cube.coords(axis="y"), []) |
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 think it's still worth testing the "find by axis" approach,
but the second part showing "rename to something inappropriate + check it doesn't affect the stated axis" obviously won't work any more.
Actually you probably can still usefully demonstrate something like this, but not by renaming the MeshCoord -- you will have to adjust the coord in the underlying sample mesh instead.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6405 +/- ##
==========================================
+ Coverage 89.80% 89.85% +0.04%
==========================================
Files 90 90
Lines 23755 24058 +303
Branches 4418 4479 +61
==========================================
+ Hits 21334 21618 +284
- Misses 1672 1685 +13
- Partials 749 755 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 Pull Request
Description
Closes #4757, continues #6125.
This pull request aims to make MeshCoords immutable, and when it's corresponding Mesh updates, for the MeshCoord to refresh its Points, Bounds, and Metadata from the Mesh.
Notes to self
node
had no bounds, has been solved.bounds
is set to an error message aboutbounds_dm
, but it seems like that's a red herring, and is solved within__init__
.