Skip to content
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

conventions: decode unsigned integers to signed if _Unsigned=false #4966

Merged
merged 4 commits into from
Mar 12, 2021

Conversation

d70-t
Copy link
Contributor

@d70-t d70-t commented Feb 26, 2021

netCDF3 doesn't know unsigned while OPeNDAP doesn't know signed (bytes).
Depending on which backend source is used, the original data is stored
with the wrong signedness and needs to be decoded based on the _Unsigned
attribute. While the netCDF3 variant is already implemented, this commit
adds the symmetric case covering OPeNDAP.

netCDF3 doesn't know unsigned while OPeNDAP doesn't know signed (bytes).
Depending on which backend source is used, the original data is stored
with the wrong signedness and needs to be decoded based on the _Unsigned
attribute. While the netCDF3 variant is already implemented, this commit
adds the symmetric case covering OPeNDAP.

closes pydata#4954
@d70-t d70-t marked this pull request as ready for review February 26, 2021 12:22
@max-sixty
Copy link
Collaborator

Would anyone who knows this area better than me be able to review @pydata/xarray ?

@shoyer
Copy link
Member

shoyer commented Mar 12, 2021

In this _Unsigned='false' convention documented anywhere?

That said, this seems safe enough...

@d70-t
Copy link
Contributor Author

d70-t commented Mar 12, 2021

I don't know if this qualifies as "documentation", but according to this merged PR on the netcdf-c sources, this is how the thredds OPeNDAP server behaves, from which they conclude that netCDF should behave accordingly. I confirmed myself that this also is how my currently installed netCDF-C behaves.

@shoyer
Copy link
Member

shoyer commented Mar 12, 2021

I don't know if this qualifies as "documentation", but according to this merged PR on the netcdf-c sources, this is how the thredds OPeNDAP server behaves, from which they conclude that netCDF should behave accordingly. I confirmed myself that this also is how my currently installed netCDF-C behaves.

OK, that's good enough for me!

@dcherian
Copy link
Contributor

Thanks @d70-t

@dcherian dcherian merged commit d4b7a60 into pydata:master Mar 12, 2021
@d70-t d70-t deleted the signed_opendap_bytes branch March 12, 2021 14:21
dcherian added a commit to dcherian/xarray that referenced this pull request Mar 18, 2021
…indow

* upstream/master:
  Fix regression in decoding large standard calendar times (pydata#5050)
  Fix sticky sidebar responsiveness on small screens (pydata#5039)
  Flexible indexes refactoring notes (pydata#4979)
  add a install xarray step to the upstream-dev CI (pydata#5044)
  Adds Dataset.query() method, analogous to pandas DataFrame.query() (pydata#4984)
  run tests on python 3.9 (pydata#5040)
  Add date attribute to datetime accessor (pydata#4994)
  📚 New theme & rearrangement of the docs (pydata#4835)
  upgrade ci-trigger to the most recent version (pydata#5037)
  GH5005 fix documentation on open_rasterio (pydata#5021)
  GHA for automatically canceling previous CI runs (pydata#5025)
  Implement GroupBy.__getitem__ (pydata#3691)
  conventions: decode unsigned integers to signed if _Unsigned=false (pydata#4966)
  Added support for numpy.bool_ (pydata#4986)
  Add additional str accessor methods for DataArray (pydata#4622)
dcherian added a commit to dcherian/xarray that referenced this pull request Mar 23, 2021
…-tasks

* upstream/master:
  Fix regression in decoding large standard calendar times (pydata#5050)
  Fix sticky sidebar responsiveness on small screens (pydata#5039)
  Flexible indexes refactoring notes (pydata#4979)
  add a install xarray step to the upstream-dev CI (pydata#5044)
  Adds Dataset.query() method, analogous to pandas DataFrame.query() (pydata#4984)
  run tests on python 3.9 (pydata#5040)
  Add date attribute to datetime accessor (pydata#4994)
  📚 New theme & rearrangement of the docs (pydata#4835)
  upgrade ci-trigger to the most recent version (pydata#5037)
  GH5005 fix documentation on open_rasterio (pydata#5021)
  GHA for automatically canceling previous CI runs (pydata#5025)
  Implement GroupBy.__getitem__ (pydata#3691)
  conventions: decode unsigned integers to signed if _Unsigned=false (pydata#4966)
  Added support for numpy.bool_ (pydata#4986)
  Add additional str accessor methods for DataArray (pydata#4622)
  add polyval to polyfit see also (pydata#5020)
  mention map_blocks in the docstring of apply_ufunc (pydata#5011)
  Switch backend API to v2 (pydata#4989)
  WIP: add new backend api documentation (pydata#4810)
  pin netCDF4=1.5.3 in min-all-deps (pydata#4982)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handling of signed bytes from OPeNDAP via pydap
4 participants