-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add support for anemoi-datasets #383
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #383 +/- ##
===========================================
- Coverage 89.75% 89.63% -0.13%
===========================================
Files 119 119
Lines 7515 7513 -2
Branches 662 662
===========================================
- Hits 6745 6734 -11
- Misses 640 649 +9
Partials 130 130 ☔ View full report in Codecov by Sentry. |
@@ -207,7 +207,7 @@ def _get_custom_key(self, key, default=None, raise_on_missing=True, **kwargs): | |||
if self._is_custom_key(key): | |||
try: | |||
if key == DATETIME: | |||
return self._valid_datetime() | |||
return self._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.
It feels a little strange to return a str rather than a datetime here. Could the client (anemoi-datasets) not perform the conversion to string?
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 requires further discussion.
return Remapping(mapping) | ||
|
||
return mapping | ||
|
||
|
||
class Patch(dict): |
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.
Would be nice to have a little description of what Patch is for
from earthkit.data.core.fieldlist import FieldList | ||
|
||
|
||
class FieldArray(FieldList): |
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.
It's not clear to me why we need this, it does not seem to be used in earthkit.data. Is it used directly by anemoi-datasets, and what does it address that the current FieldList does not?
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.
With it you can create a fieldlist out of a set/list of fields. FieldList does not store a list of fields (it is more generic) so cannot do it.
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.
It is used directly in anemoi-datasets.
return self.metadata.get("gridName") | ||
|
||
def mars_area(self): | ||
north = self.metadata.get("latitudeOfFirstGridPointInDegrees") |
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.
Not all grids will have these metadata keys (e.g. Lambert), but I guess we'd want to fail for these ones anyway
This PR adds support for anemoi-datasets.
Major changes
for a latlon grid returns [dx,dy] otherwise
metadata("gridName")
returns [north, west, south, east]
returns (lats, lons)
Added new FieldList type:
FieldArray
. Name should be reviewed since earthkit-data already hasArrayFieldList
Added the
FieldCube
andCubelet
classes. A version of it already exists in the feature/tensor branch.The "valid_datetime" metadata key now returns a
str
instead ofdatetime.datetime
Add dependency on earthkit-geo