-
Notifications
You must be signed in to change notification settings - Fork 0
Refactoring pytables out and h5py in #1
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
Conversation
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 PR performs a major refactoring to modernize the OpenMatrix Python library by replacing PyTables with h5py as the underlying HDF5 library. The changes include updating the build system, testing framework, and adding continuous integration workflows.
Key changes:
- Replaces PyTables dependency with h5py for HDF5 file operations
- Migrates from setup.py to modern pyproject.toml-based packaging
- Switches from nosetests to pytest for testing with significantly expanded test coverage
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_validator.py | New comprehensive test suite for the validator module with ~495 lines covering all validation checks |
| tests/test_file.py | New comprehensive test suite for file operations with ~598 lines achieving nearly 100% coverage |
| openmatrix/init.py | Updated to use h5py instead of PyTables, simplified imports and file opening logic |
| openmatrix/File.py | Complete rewrite to extend h5py.File instead of tables.File, replacing PyTables API calls with h5py equivalents |
| openmatrix/validator.py | Refactored to use h5py API for checking OMX file validity and format compliance |
| openmatrix/Exceptions.py | Added MappingError exception class alongside existing ShapeError |
| pyproject.toml | New modern packaging configuration defining project metadata, dependencies, and build system |
| setup.py | Removed in favor of pyproject.toml |
| .github/workflows/ci.yml | New CI workflow for linting and testing across multiple Python versions and operating systems |
| README.md | Updated documentation to reflect h5py usage instead of PyTables |
| example/python-omx-sample.py | Code formatting improvements and style consistency updates |
| CHANGES.txt | Added version 0.4.0 release entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Jake-Moss, can you review this PR when you get back so I can submit it against the main repo? It is almost a complete rewrite, but much of it should be easy to go over. |
Co-authored-by: Jake Moss <jake@outerloop.io>
Co-authored-by: Jake Moss <jake@outerloop.io>
Co-authored-by: Jake Moss <jake@outerloop.io>
Co-authored-by: Jake Moss <jake@outerloop.io>
Co-authored-by: Jake Moss <jake@outerloop.io>
We can assume that once an OMX file is open, it's in a valid state, otherwise we should loudly complain
|
At this point I think I'll need a review from you @pedrocamargo haha. I also brought the codebase up to 100% branch coverage for both the codebase and test suite. https://github.com/pedrocamargo/omx-python/actions/runs/20740677926?pr=1 Should also reuse some of the old doc strings, but a few need to be updated |
Refactoring pytables out and h5py in
This PR contains the following items: