pik-copan / pyunicorn

Unified Complex Network and Recurrence Analysis Toolbox
http://pik-potsdam.de/~donges/pyunicorn/
Other
195 stars 87 forks source link

MAINT: Refactor caching (#124, #148, #153) #219

Closed ntfrgl closed 7 months ago

ntfrgl commented 7 months ago

This PR fully implements:

Obviously, there is still more refactoring to be done to make the caching system consistent across the project, but this PR should deliver on the main concern of #148, namely a well-behaved caching decorator. The last commit message should provide enough context for reviewing. @fkuehlein, please raise any concerns you might have, since you are expected to maintain this code going forward.

codecov[bot] commented 7 months ago

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (2739894) 62.82% compared to head (415e2bf) 63.76%.

Files Patch % Lines
src/pyunicorn/climate/havlin.py 52.00% 12 Missing :warning:
src/pyunicorn/timeseries/recurrence_plot.py 85.89% 11 Missing :warning:
src/pyunicorn/core/cache.py 85.71% 8 Missing :warning:
src/pyunicorn/timeseries/joint_recurrence_plot.py 50.00% 5 Missing :warning:
src/pyunicorn/climate/climate_network.py 80.00% 4 Missing :warning:
src/pyunicorn/climate/hilbert.py 73.33% 4 Missing :warning:
src/pyunicorn/core/network.py 96.77% 4 Missing :warning:
src/pyunicorn/climate/mutual_info.py 76.92% 3 Missing :warning:
src/pyunicorn/climate/climate_data.py 95.83% 1 Missing :warning:
src/pyunicorn/core/geo_network.py 90.90% 1 Missing :warning:
... and 1 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #219 +/- ## ========================================== + Coverage 62.82% 63.76% +0.93% ========================================== Files 43 44 +1 Lines 6220 6221 +1 ========================================== + Hits 3908 3967 +59 + Misses 2312 2254 -58 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fkuehlein commented 7 months ago

Thanks a lot @ntfrgl for all the work put into implementing this and also for all the little cleanups and refactors along the way!

I took a bit of time to understand the structure and functionality of the new Cached class in detail, although its application appears straightforward indeed. I think I'll be able to maintain it, continuously include it in further classes and eventually add subclass-specific tests.

I added the remaining minor fixes in the commits above to make sure all checks pass, so this PR should be good to go.

ntfrgl commented 7 months ago

Thank you for the careful review. I'm glad to know the project in your hands.

As a last addition, I slightly simplified the implementation of nested Cached objects in afc6abf, effectively removing some debugging artifacts. But that shouldn't concern the current release.