Fixes to datacube extension helper #51
Conversation
|
@huard your review would be helpful here |
STACpopulator/extensions/datacube.py
Outdated
| try: | ||
| attrs = meta["attributes"] | ||
| except KeyError: | ||
| # Some variables like "time_bnds" in some model files do not have any attributes. | ||
| attrs = {} |
There was a problem hiding this comment.
| try: | |
| attrs = meta["attributes"] | |
| except KeyError: | |
| # Some variables like "time_bnds" in some model files do not have any attributes. | |
| attrs = {} | |
| attrs = meta.get("attributes", {}) |
| def _infer_variable_units_description(self, name, attrs): | ||
| """Try to infer the units and description of some simple coordinate variables.""" | ||
| if name == "time_bnds": | ||
| related_variable = "time" | ||
| attrs["description"] = "bounds for the time coordinate" | ||
| elif name == "lat_bnds": | ||
| related_variable = "lat" | ||
| attrs["description"] = "bounds for the latitude coordinate" | ||
| elif name == "lon_bnds": | ||
| related_variable = "lon" | ||
| attrs["description"] = "bounds for the longitude coordinate" | ||
| else: | ||
| return |
There was a problem hiding this comment.
I suspect you'd be able to harden the bound variables inference using cf-xarray. It looks for the bounds attributes of variables. See https://cfconventions.org/cf-conventions/cf-conventions.html#cell-boundaries
There was a problem hiding this comment.
There was a problem hiding this comment.
Now I start to remember. We didn't want to add cf-xarray as a dependency, right? So we just copied some of the logic here. Either we bring more of the cf logic into the repo, or add is as a dependency ? No strong opinion.
There was a problem hiding this comment.
Ok, then we can keep it cf-array free for now. I think this will be the path of least resistance, and time is a bit of an issue at the moment.
There was a problem hiding this comment.
Sure, if the fix works for your datasets, all good. I'm just saying don't rewrite cf-xarray if you hit corner cases and need to make that logic more comprehensive.
There was a problem hiding this comment.
@david now all the bounds variables are shown as auxiliary
There was a problem hiding this comment.
@huard is in better position to answer regarding these attributes. Will let it up to both of you.
time_bnds) do not have variable "attributes" in certain model files. As a result the extension code crashes. 8621b6d fixes that.An example of how the data cube extension looks like with these changes is here.
An outstanding issue is that
time_bndsvariable is still not being inferred as an auxiliary variable. This will require a hack since this variable cannot be determined to be auxiliary using the framework currently in place (because the variable in the netCDF file will not contain any of the "criteria" that could be put into place for the benefit of theis_coordinate).