Assert that Fields in FieldSet have the same calendar for their time domain#2002
Merged
VeckoTheGecko merged 5 commits intov4-devfrom May 7, 2025
Merged
Assert that Fields in FieldSet have the same calendar for their time domain#2002VeckoTheGecko merged 5 commits intov4-devfrom
VeckoTheGecko merged 5 commits intov4-devfrom
Conversation
This ensures that all fields in a FieldSet always have calendars that are compatible with each other This commit also adds CalendarError, DatetimeLike, and time helpers
Contributor
Author
|
I still need to fix some tests related to a prior PR (accidentally merged when |
VeckoTheGecko
commented
May 7, 2025
Comment on lines
+169
to
+175
| try: | ||
| self.time_interval = get_time_interval(data) | ||
| except ValueError as e: | ||
| e.add_note( | ||
| f"Error getting time interval for field {name!r}. Are you sure that the time dimension on the xarray dataset is stored as datetime or cftime datetime objects?" | ||
| ) | ||
| raise e |
Contributor
Author
There was a problem hiding this comment.
nicer error message ;)
erikvansebille
approved these changes
May 7, 2025
fluidnumericsJoe
approved these changes
May 7, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v4-devfor v4 changes)For a given FieldSet, all Fields need to define time in the same way. It doesn't make sense to run a simulation where one Field is defined using a 365 day calendar while another is defined on a 360 day calendar as Parcels wouldn't know what to do with the mismatched days.
This PR:
add_fieldmethod assertions that the fields are on the same calendar (and also adds tests for this)VectorField.time_interval(which should have need done in AddField.time_intervalattribute #2000)It is the users responsibility that they provide xarray datasets that have the same calendars. If their data doesn't have the same calendar, they can use xarray to modify them in place before providing them to Parcels.