-
Notifications
You must be signed in to change notification settings - Fork 17
fix: Fix time metadata #188
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -589,12 +589,12 @@ class NewValidDateTimeField(NewMetadataField): | |
| """ | ||
|
|
||
| def __init__(self, field: Any, valid_datetime: Any) -> None: | ||
| date = int(valid_datetime.date().strftime("%Y%m%d")) | ||
| assert valid_datetime.time().minute == 0, valid_datetime | ||
| time = valid_datetime.time().hour | ||
| date = int(valid_datetime.strftime("%Y%m%d")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marek what's the current error that you're getting if those changes are not applied when using your recipe with repeated dates? Wondering if this an issue in transforms or that repeated dates is not returning the right valid_datetime format
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope this message helps to understand the situation. A bit of debugging revealed that The issue then propagates to the variable grouping around So far, I've already solved the I was wondering, if the format and unit of When it comes to the |
||
| time = int(valid_datetime.strftime("%H%M")) | ||
|
|
||
| self.valid_datetime = valid_datetime | ||
|
|
||
| # is 0 a valid step value here!?! | ||
| super().__init__(field, date=date, time=time, step=0, valid_datetime=valid_datetime.isoformat()) | ||
|
|
||
|
|
||
|
|
||
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.
could we still have an assert to check minutes need to be equal to 0 or why we want to support mins!=0?
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 believe there are use cases of sub-hourly data and support was added to anemoi-datasets a while ago. However, I'm nut sure, if they are supposed to be combined with repeated-dates. So I though this assert was an oversight.