Switch to CLI11 and addition of a cell metadata runmode#68
Conversation
|
Thanks @atchox ! can you describe the specific tests added for the cluster-aware use mode? |
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the yamet command-line interface and improves R portability by replacing boost::program_options with CLI11 and refactoring print functions to use output streams. The PR also removes the R package bindings and adds support for metadata files with cell clustering.
Changes:
- Replaced boost::program_options with CLI11 for command-line argument parsing
- Updated print functions to accept
std::ostream¶meters instead of writing directly to std::cout for better R portability - Added metadata file support with cluster-based aggregation capabilities
- Removed R package bindings (yamet-r directory) and build infrastructure
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| yamet-r/src/bindings.cpp | Deleted - R bindings removed |
| yamet-r/src/.clang-format | Deleted - R package configuration removed |
| yamet-r/configure | Deleted - R package build script removed |
| yamet-r/build.sh | Deleted - R package build script removed |
| yamet-r/R/yamet.R | Deleted - R wrapper function removed |
| yamet-r/NAMESPACE | Deleted - R package namespace removed |
| yamet-r/DESCRIPTION | Deleted - R package metadata removed |
| yamet-r/.Rbuildignore | Deleted - R package configuration removed |
| method/src/reference.cpp | Updated print method to accept ostream parameter |
| method/src/main.cpp | Migrated to CLI11, added metadata/cluster support |
| method/src/intervals.cpp | Updated print method to accept ostream parameter |
| method/src/file_map.cpp | Updated print/export methods for ostream and cluster support |
| method/src/cli_config.cpp | New file implementing CLI11-based argument parsing |
| method/src/boost.cpp | Deleted - replaced by cli_config.cpp |
| method/src/align.cpp | Added metadata parsing and FilesMeta support |
| method/include/lib/file_stream.h | Made detectCompression static |
| method/include/lib/file_classes.h | Added FileMeta structure and cluster support in aggregation |
| method/include/lib/chrData.h | Updated print method signatures to accept ostream |
| method/include/lib/align.h | Updated to use FilesMeta instead of string vectors |
| method/include/cli/cli_config.h | New header for CLI configuration structure |
| method/include/cli/boost.h | Deleted - replaced by cli_config.h |
| method/config/version.h.in | Added PROJECT_DESCRIPTION macro |
| method/CMakeLists.txt | Updated dependencies from boost::program_options to CLI11 |
| .github/workflows/test.yml | Updated to install CLI11 and use new CLI syntax |
| .github/workflows/tar.yml | Updated to install CLI11 dependency |
| .github/workflows/memleaks.yml | Updated to install CLI11 and use new CLI syntax |
| .github/workflows/debug.yml | Updated to install CLI11 and use new CLI syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::error_code ec; | ||
| if (!std::filesystem::exists(filepath_path, ec)) { | ||
| throw std::system_error( | ||
| ENOENT, std::generic_category(), | ||
| "in metadata line " + std::to_string(line_num) + "\n\n\t\033[33m" + line + | ||
| "\033[0m\n\nreferenced file does not exist: " + filepath_path.string()); | ||
| } |
There was a problem hiding this comment.
The error_code ec is passed to std::filesystem::exists but is never checked. If the filesystem operation itself fails (e.g., permission denied on parent directory), the error would be stored in ec but the code would still throw based on the return value of exists. Consider checking ec and throwing a more specific error if the filesystem operation failed rather than just treating it as "file doesn't exist".
don't have a test for that currently – will add something now |
|
added a test suite at b9a9beb generated from bruijnmeth |
well, it is longer "mostly CLI updates"
Changes
boost::program optionswithCLI11ostreamsfor R portability--metadatamode for working with clusters efficiently--cellalso accepts a container of containers as defined here in the CLI11 docs