-
Notifications
You must be signed in to change notification settings - Fork 0
Add memoization for invoking callbacks #110
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
Add memoization for invoking callbacks #110
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 89.39% 92.53% +3.13%
==========================================
Files 84 85 +1
Lines 1443 1460 +17
Branches 217 220 +3
==========================================
+ Hits 1290 1351 +61
+ Misses 124 91 -33
+ Partials 29 18 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
- Why does this work? Please comment.
- Global variable used instead of local memoization
- Why time-consuming JSON conversion instead of suggested shallow comparison
- Missing unit test
This seems to be an insufficient solution as the key used into the map of memoized values is the value serialized itself. But the key should be an ID created from the propertyRef and the value should be the array of input values.
Co-authored-by: Norman Fomferra <[email protected]>
Closes #112
This PR also closes xcube-dev/xcube-viewer#490