-
Notifications
You must be signed in to change notification settings - Fork 67
[Feat] [Py] Refactor Python binding #275
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
base: develop
Are you sure you want to change the base?
Conversation
dfc671b
to
a5d7c1e
Compare
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.
Pull Request Overview
This pull request refactors the Python bindings for libCacheSim to simplify the registration of different replacement algorithms and add analyzer bindings. The refactoring replaces the monolithic C++ binding file with a modular approach organized across multiple files, each handling specific functionality areas.
- Splits the large
pylibcachesim.cpp
into specialized modules for better maintainability - Introduces new Python wrapper classes that provide a unified interface for cache algorithms
- Adds analyzer bindings to enable trace analysis functionality
Reviewed Changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
scripts/install_python_dev.sh | Updates pip command to use python -m pip for better compatibility |
libCacheSim-python/src/pylibcachesim.cpp | Removes the entire monolithic C++ binding implementation |
libCacheSim-python/src/export_*.cpp | New modular C++ export files for cache, reader, analyzer, and misc functionality |
libCacheSim-python/libcachesim/*.py | New Python wrapper modules providing clean interfaces |
libCacheSim-python/tests/* | Removes existing test files (likely to be replaced) |
Comments suppressed due to low confidence (2)
libCacheSim-python/libcachesim/cache.py:354
- [nitpick] The function name 'nop_method' is unclear. Consider renaming to 'default_hook' or 'no_op_hook' to better indicate its purpose as a default hook function.
def nop_method(*args, **kwargs):
libCacheSim-python/src/export.cpp:17
- [nitpick] The module name 'libcachesim_python' contains an underscore which may not follow Python naming conventions. Consider using 'libcachesim' or 'libcachesim_py' for consistency.
PYBIND11_MODULE(libcachesim_python, m) {
a5d7c1e
to
9d0b8a7
Compare
5c3ffb7
to
0fff176
Compare
da172ef
to
7f11a0a
Compare
In order to substantially improve the maintainability, we make a huge refactor inspired by the project taichi,
TODO:
reader_protocol
and mark it should be removed once we have streaming synthetic reader at c++ side