fix: Complete VSRI implementation and improve code quality#150
Open
fix: Complete VSRI implementation and improve code quality#150
Conversation
VSRI Fixes: - Fix float division bugs in get_sample() and fits_segment() - Improve error handling in load() - replace unwraps with proper error propagation - Document MAX_INDEX_SAMPLES as configurable - Fix doc test imports and empty line after doc comment Code Quality Improvements: - Replace BinConfig struct with bincode_config() function - Remove unused POLYNOMIAL_COMPRESSOR_ID and IDW_COMPRESSOR_ID constants - Remove unused vsri import from main.rs - Fix clippy warnings (needless borrows, into_iter on ref) All tests passing (85 tests total) Clippy clean with -D warnings
Fixes identified in PR #150 review: 1. **VSRI Frame Tagging**: CompressorFrame::compress_vsri now correctly sets self.compressor = Compressor::VSRI so VSRI data is properly tagged in serialized files 2. **VSRI Decompression**: - Added CompressorFrame::decompress_vsri() method - Added CompressedStream::decompress_vsri() method - Added CompressorFrame::is_vsri() helper method - VSRI files can now be round-tripped successfully 3. **CLI Safety**: Removed VSRI from CompressorType enum to prevent users from selecting --compressor=vsri which would cause panic - VSRI is for internal timestamp compression only - Not applicable to f64 data compression 4. **Better Error Handling**: Changed Compressor::decompress VSRI case from panic to log warning and return empty vec, allowing graceful handling if VSRI frame is accidentally processed as f64 Testing: - Added test_vsri_roundtrip test to verify complete cycle - All 86 tests passing - Clippy clean with -D warnings
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
1 participant
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.
Summary
Completes the VSRI (Very Small Rolo Index) implementation by fixing critical bugs and improving code quality across the codebase.
VSRI Fixes
Critical Bug Fixes
get_sample()andfits_segment()load()functionDocumentation & Configuration
MAX_INDEX_SAMPLESas configurableCode Quality Improvements
BinConfigstruct withbincode_config()functionPOLYNOMIAL_COMPRESSOR_ID,IDW_COMPRESSOR_IDvsriimport from main.rsTesting
-D warningsRelated Issues
Addresses VSRI completion as part of repository modernization (#7, #9, #11 related)