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.

Sign up for GitHub

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

Jump to bottom

implement isnull using full_like instead of zeros_like #7395

Merged
merged 6 commits into from
Jan 23, 2024
Merged

implement isnull using full_like instead of zeros_like #7395

merged 6 commits into from
Jan 23, 2024

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Dec 20, 2022

After changing the behavior of the implementation of *_like in pint, it seems comparisons fail now. As it turns out, this is because we're using zeros_like to return all-False arrays from isnull for input with non-nullable dtypes.

I'd argue that

full_like(data, dtype=bool, fill_value=False)

is a little bit easier to understand than

zeros_like(data, dtype=bool)

as the latter requires knowledge about the bit representation of False, so the change is not only to get pint to work properly.

  • Tests added

@github-actions github-actions bot added the topic-arrays related to flexible array support label Dec 20, 2022
@keewis
Copy link
Collaborator Author

keewis commented Dec 21, 2022

The test failures are real, unfortunately. pint's implementation of full_like explicitly defines the parameters, among them subok which the dask implementation does not support.

That means that update strategies might become tricky: the new version of pint would change the behavior and require us to implement isnull using full_like, but a bug in pint prevents that from working on dask. So not sure how to proceed?

One way might be to have pint issue a bugfix release first, wait on a release of xarray that contains this change, then release a version with the change in behavior (or deprecate / allow switching between the old / new behavior).

@dcherian
Copy link
Contributor

So not sure how to proceed?

I think it'd be OK to have some compatibility code with hardcoded version numbers. It's simpler and more user-friendly than the other alternatives.

@keewis keewis mentioned this pull request Apr 14, 2023
Open
5 tasks
@dcherian dcherian added plan to merge Final call for comments and removed needs work labels Jan 18, 2024
@dcherian dcherian merged commit d34ebff into pydata:main Jan 23, 2024
30 checks passed
@keewis keewis deleted the isnull-nonnullable branch January 28, 2024 14:19
andersy005 added a commit to TomNicholas/xarray that referenced this pull request Jan 30, 2024
@andersy005
Merge branch 'main' into namedarray-parallelcompat
53906e4 
* main: (153 commits)
  Add overloads to get_axis_num (pydata#8547)
  Fix CI: temporary pin pytest version to 7.4.* (pydata#8682)
  Bump the actions group with 1 update (pydata#8678)
  [namedarray] split `.set_dims()` into `.expand_dims()` and `broadcast_to()` (pydata#8380)
  Add chunk-friendly code path to `encode_cf_datetime` and `encode_cf_timedelta` (pydata#8575)
  Fix NetCDF4 C version detection (pydata#8675)
  groupby: Don't set `method` by default on flox>=0.9 (pydata#8657)
  Fix automatic broadcasting when wrapping array api class (pydata#8669)
  Fix unstack method when wrapping array api class (pydata#8668)
  Fix `variables` arg typo in `Dataset.sortby()` docstring (pydata#8670)
  dt.weekday_name - removal of function (pydata#8664)
  Add `dev` dependencies to `pyproject.toml` (pydata#8661)
  CI: Pin scientific-python/upload-nightly-action to release sha (pydata#8662)
  Update HOW_TO_RELEASE.md by clarifying where RTD build can be found (pydata#8655)
  ruff: use extend-exclude (pydata#8649)
  new whats-new section (pydata#8652)
  xfail another test on windows (pydata#8648)
  use first element of residual in _nonpolyfit_1d (pydata#8647)
  whatsnew for v2024.01.1
  implement `isnull` using `full_like` instead of `zeros_like` (pydata#7395)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
plan to merge Final call for comments topic-arrays related to flexible array support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@keewis @dcherian @headtr1ck