-
Notifications
You must be signed in to change notification settings - Fork 55
Avoid converting chunked data to Numpy arrays (i.e. .values calls)
#1588
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
Open
cmdupuis3
wants to merge
26
commits into
UXARRAY:main
Choose a base branch
from
cmdupuis3:cmd/devalue
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
5ddf672
Devalue: dataarray.py
cmdupuis3 995044f
Devalue: dataset.py
cmdupuis3 7ea51bb
Devalue: aggregation.py (includes dask branches)
cmdupuis3 e4f007d
Devalue: dataarray_accessor.py
cmdupuis3 2fd47f8
Devalue: grid.py
cmdupuis3 d161efa
Devalue: slice.py
cmdupuis3 396f9bf
Devalue: dataarray.py branch on numpy/dask
cmdupuis3 1408c20
Devalue: replace with .data cases
cmdupuis3 06149a9
Devalue: last bits?
cmdupuis3 6cf056a
Devalue: linting
cmdupuis3 8137ee6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 436dab2
Merge branch 'main' into cmd/devalue
cmdupuis3 5f010b2
devalue linting
cmdupuis3 5ee5157
Merge branch 'main' into cmd/devalue
erogluorhan 850d4ac
devalue: import and output type cleanup
cmdupuis3 f89c41c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] af76639
devalue: rename dask/numpy repro tests
cmdupuis3 84c206b
Merge branch 'main' into cmd/devalue
cmdupuis3 8e86ae9
devalue: Move dask imports back inside
cmdupuis3 29f13e8
Merge branch 'main' into cmd/devalue
rajeeja 9cfd53e
merge main into cmd/devalue
cmdupuis3 1150710
Merge branch 'main' into cmd/devalue
cmdupuis3 0d92b33
Fix committed conflict markers in test_basic.py
cmdupuis3 180345e
Devalue: type-check in tests
cmdupuis3 1cafb3f
Devalue: stricter type tests
cmdupuis3 83ca8de
Devalue: dask/numpy repro of xr.dot
cmdupuis3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Remove the
importorskipcalls everywhere; tests should crash if dask is not installed, not be skipped silently (well, mostly silently). Existing tests currently onmainalready use dask without importorskip, and the CI for the test suite installs dask, so it should be safe to assume dask is available.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.
If we're assuming that dask is available for these tests though, why can't we assume that in the source code? My strategy lately has been to hedge with the assumption that dask will eventually be optional at the package level, but you're saying here I should be moving in the opposite direction?