[Feat] Complete process_trace and reader_init_param#271
Merged
haochengxia merged 8 commits intodevelopfrom Jul 18, 2025
Merged
Conversation
3c36c20 to
9e6b3e0
Compare
9e6b3e0 to
e29a9e9
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request completes the implementation of process_trace functionality and reader_init_param bindings, along with adding Python linting support. The PR makes process_trace return both object and byte miss ratios, completes the ReaderInitParam bindings with full parameter support, and adds ruff-based Python code quality checks to the pre-commit hooks.
Key Changes
- Enhanced process_trace to return both object and byte miss ratios instead of just object miss ratio
- Completed ReaderInitParam bindings with comprehensive parameter support and multiple constructor options
- Added ruff linting and formatting checks for Python files in the pre-commit hook system
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup_hooks.sh | Added Python file detection and ruff linting/formatting checks to pre-commit hooks |
| libCacheSim-python/src/pylibcachesim.cpp | Enhanced process_trace functions to return tuple of miss ratios and completed ReaderInitParam bindings |
| libCacheSim-python/libcachesim/eviction.py | Updated process_trace method signatures to return tuple of miss ratios |
| libCacheSim-python/tests/ | Updated test files to handle new tuple return values from process_trace |
| libCacheSim-python/libcachesim/init.py | Added ReaderInitParam to exports and reorganized all list |
| Documentation files | Updated examples and documentation to reflect new process_trace return format |
1a1a11a
approved these changes
Jul 18, 2025
| if not isinstance(reader, Reader): | ||
| # streaming generator | ||
| if isinstance(reader, (_ZipfRequestGenerator, _UniformRequestGenerator)): | ||
| if isinstance(reader, _GENERATOR_TYPES): |
Owner
There was a problem hiding this comment.
In the ideal world, I hope we have streaming reader in C as well, but let's live with it
| if not isinstance(reader, Reader): | ||
| # streaming generator | ||
| if isinstance(reader, (_ZipfRequestGenerator, _UniformRequestGenerator)): | ||
| if isinstance(reader, _GENERATOR_TYPES): |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
reader_init_param