Add clear_cache and clear_task_cache methods to BaseDataset #770
+191
−7
Conversation
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
Implements sunlabuiuc#765 by adding two new methods to manage cache cleanup: - clear_cache(): Clears entire dataset cache including global event dataframe and all task caches - clear_task_cache(task=None): Clears only the specified task's cache while preserving global event cache and other task caches Both methods handle non-existent caches gracefully and provide comprehensive logging.
EricSchrock
requested changes
Jan 4, 2026
Collaborator
EricSchrock
left a comment
EricSchrock
left a comment
There was a problem hiding this comment.
Could you add tests for these new methods to tests/core/test_caching.py, both to prove it works as expected and to act as example code?
Logiquo
reviewed
Jan 4, 2026
Logiquo
reviewed
Jan 4, 2026
11 tasks
- Extract task cache directory path generation into _get_task_cache_dir() helper method for consistency between set_task() and clear_task_cache() - Update clear_task_cache() to use the helper method - Clarify in docstring that clear_task_cache() only clears default cache location, not custom cache_dir paths - Add 5 comprehensive tests to tests/core/test_caching.py: * test_clear_cache_removes_all_caches * test_clear_cache_handles_nonexistent_cache * test_clear_task_cache_removes_only_specified_task * test_clear_task_cache_handles_nonexistent_cache * test_get_task_cache_dir_consistency Addresses review feedback from @Logiquo and @EricSchrock on PR sunlabuiuc#770
Collaborator
|
CI has failed. |
Store cache paths as strings before calling clear methods to prevent the cache_dir property from recreating directories when accessed after clearing. This ensures the tests correctly verify that caches are removed. Fixes CI test failures in test_clear_cache_removes_all_caches and test_clear_task_cache_removes_only_specified_task.
Logiquo
added
core
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Implements #765 by adding two new methods to manage cache cleanup:
Both methods handle non-existent caches gracefully and provide comprehensive logging.