Skip to content
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

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

added functional timestamps #6125

wants to merge 16 commits into from

Conversation

ESadek-MO
Copy link
Contributor

@ESadek-MO ESadek-MO commented Aug 19, 2024

🚀 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.

Copy link
Contributor

@trexfeathers trexfeathers left a 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

lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Show resolved Hide resolved
lib/iris/mesh/components.py Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
lib/iris/mesh/components.py Outdated Show resolved Hide resolved
Comment on lines +2061 to +2064
def test_mesh_timestamp(self):
result = self.cube.mesh.timestamp
self.assertNotNone(result)

Copy link
Contributor

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 Show resolved Hide resolved
lib/iris/mesh/components.py Show resolved Hide resolved
Comment on lines 2775 to 2785
@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)
Copy link
Contributor

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.

lib/iris/mesh/components.py Outdated Show resolved Hide resolved
msg = "Cannot yet create a MeshCoord without points."
raise ValueError(msg)

# Get the 'coord identity' metadata from the relevant node-coordinate.
Copy link
Contributor

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

@ESadek-MO ESadek-MO self-assigned this Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Ensure MeshCoord points and bounds always agree with the Mesh (while keeping Mesh mutable)
2 participants