-
Notifications
You must be signed in to change notification settings - Fork 284
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
added functional timestamps #6125
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
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.
Thanks @ESadek-MO
I'm confident that you have captured all the manager methods that need to update the timestamp.
Some requests for you
for more information, see https://pre-commit.ci
def test_mesh_timestamp(self): | ||
result = self.cube.mesh.timestamp | ||
self.assertNotNone(result) | ||
|
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.
This is an inappropriate place for testing timestamp
- it has no bearing on the functioning of a Cube
.
lib/iris/mesh/components.py
Outdated
@property | ||
def bounds(self): | ||
if self.timestamp < self._mesh.timestamp or self.timestamp is None: | ||
_, self.bounds, _ = self._load_points_and_bounds() | ||
return super.bounds | ||
|
||
@bounds.setter | ||
def bounds(self, value): | ||
if len(value) > 0 and self.bounds: | ||
msg = "Cannot set 'bounds' on a MeshCoord." | ||
raise ValueError(msg) |
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.
These two together create some kind of infinite loop, since the setter accesses the getter then the getter accesses the setter...
Also when I tested this, asking for my_coord.bounds
returns None
, but perhaps that problem will go away once the above problem is fixed.
msg = "Cannot yet create a MeshCoord without points." | ||
raise ValueError(msg) | ||
|
||
# Get the 'coord identity' metadata from the relevant node-coordinate. |
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.
Something missed in #4757 and in previous discussions: the metadata such as units
, standard_name
, etcetera is also subject to change.
Thankfully it is quick to recalculate, so we don't need to protect it behind a date check. But we do need a way to make sure it is recalculated on the fly.
Metadata classes a tricksy and it's probably best if we don't make any changes there. Instead we can put self._metadata_manager
behind a `@property' so that we can regenerate it every time before returning it:
1
- self._metadata_manager = metadata_manager_factory(MeshCoordMetadata)
+ self._some_other_name = metadata_manager_factory(MeshCoordMetadata)
2
@property
def _metadata_manager(self):
# An explanatory comment.
self._some_other_name.standard_name = something
# Etcetera for all standard coord metadata
# THIS INCLUDES DETERMINING THE CORRECT VALUE, AS IN
# THE CURRENT BLOCK WITHIN _load_points_and_bounds
return self._some_other_name
3
I don't think this will be necessary since I don't think there will be any calls to set self._metadata_manager
?
@_metadata_manager.setter
def _metadata_manager(self, value):
self._some_other_name = value
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
🚀 Pull Request
Closes #4757.
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.