Fix decode_cf_variable. #153
Conversation
It was still using da.data instead of da.values
|
I'm not opposed to this in principle, but this is an internal API that wasn't really for external consumption. If you're relying on this in external code, then we should think about what the public API should be and add tests there, not for this internal function. |
| @@ -413,7 +413,9 @@ def get_to(source, dest, k): | |||
|
|
|||
| def decode_cf_variable(var, concat_characters=True, mask_and_scale=True, | |||
| decode_times=True): | |||
| # use _data instead of data so as not to trigger loading data | |||
| if isinstance(var, xray.DataArray): | |||
| var = var.variable | |||
There was a problem hiding this comment.
I would consider using var = xray.variable.as_variable(var) instead.
|
I'm confused, are you saying we shouldn't add tests for internal functions? |
|
No, just that it's unclear if it's a good idea to add tests (or On Thu, Jun 12, 2014 at 1:29 PM, akleeman [email protected] wrote:
|
|
So ... do you want me to remove the tests and only include the .data -> .values fix? Re: api. I'm working on a better storage scheme for reflectivity data, which involves CF decoding plus remapping some values. I could build my own data store which implements the netcdf store and adds the extra layer there .. but just using decode_cf_variable is far easier. In general, I can imagine a situation where we would want to allow users to provide their own decoding functions, but thats a larger project. |
|
OK, I guess this is fine -- I merged the other PR. I will open another issue for exposing a public interface to decoding functions. |
updates: - [github.com/keewis/blackdoc: v0.3.6 → v0.3.7](keewis/blackdoc@v0.3.6...v0.3.7) - [github.com/pre-commit/mirrors-mypy: v0.971 → v0.981](pre-commit/mirrors-mypy@v0.971...v0.981) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
decode_cf_variable was still using da.data instead of da.values.
It now also works with DataArray as input.