-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Faster interp #4740
Faster interp #4740
Conversation
3fff969
to
fbb5ba9
Compare
Thanks for opening an issue on the dask side. On the xarray side, we want to cache some of these properties, see #3514 |
Well, I think this is good enough. It's quite an improvement already and I think the error will disappear if the CI is rerun. |
Thanks for pinging @Illviljan . Looks good from my side too. @Illviljan what's that profiling tool you're using? The hierarchy of functions looks useful! |
@max-sixty It's the profiler Spyder has. It's been quite useful for hunting down various bottlenecks but it has its quirks as well. |
validated_indexers = { | ||
k: _validate_interp_indexer(maybe_variable(obj, k), v) | ||
for k, v in indexers.items() | ||
} | ||
|
||
# optimization: subset to coordinate range of the target index | ||
if method in ["linear", "nearest"]: | ||
for k, v in validated_indexers.items(): | ||
obj, newidx = missing._localize(obj, {k: v}) | ||
validated_indexers[k] = newidx[k] |
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.
validated_indexers = { | |
k: _validate_interp_indexer(maybe_variable(obj, k), v) | |
for k, v in indexers.items() | |
} | |
# optimization: subset to coordinate range of the target index | |
if method in ["linear", "nearest"]: | |
for k, v in validated_indexers.items(): | |
obj, newidx = missing._localize(obj, {k: v}) | |
validated_indexers[k] = newidx[k] | |
validated_indexers = dict() | |
method_is_localizeable = method in ["linear", "nearest"] | |
for k, v in indexers.items(): | |
validated_indexer = _validate_interp_indexer(maybe_variable(obj, k), v) | |
# optimization: subset to coordinate range of the target index: | |
if method_is_localizeable: | |
obj, newidx = missing._localize(obj, {k: validated_indexer}) | |
validated_indexer = newidx[k] | |
validated_indexers[k] = validated_indexer |
Could do something like this to avoid the mutating loops.
Test failure is real and looks like some bug with |
This reverts commit a6cd732.
OK should be good to merge. We can set the |
Co-authored-by: Illviljan <14371165+Illviljan@users.noreply.github.com>
isort . && black . && mypy . && flake8
whats-new.rst