-
-
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
Cache some properties #5540
base: main
Are you sure you want to change the base?
Cache some properties #5540
Conversation
Unit Test Results 6 files 6 suites 53m 11s ⏱️ Results for commit 59bd433. ♻️ This comment has been updated with latest results. |
Thanks for kicking this off @Illviljan . My concern with this is that adding the It also sounds like the speed-ups I was seeing might not be real. Probably we should have an ASV so there's a benchmark we're all looking at. |
The case I'm optimizing is dataset interpolation with many variables. Although it's not the bottleneck there I halved the shape time from ~180ms to ~92ms with this change. |
Resurrecting this, as discussed on the dev call. Could we replace the pandas decorator with the one from the standard library? That may require adding Then as long as the benchmarks still look good, there was consensus that we should merge. |
Cache some small properties that are rather slow to calculate but doesn't change that often.
Questions that needs to be resolved:
pre-commit run --all-files
whats-new.rst
api.rst
Notes
ndim
inNdimSizeLenMixin
cannot be easily be replaced withcache_readonly
.