Repository navigation
Read the file only once when select_streams is a list of dicts - #181
Merged
Merged
Conversation
Match each stream header against the select_streams criteria while loading instead of resolving all streams in a separate pass over the file first. Fixes xdf-modules#60.
cboulay
approved these changes
Oct 27, 2026
cboulay
left a comment
cboulay
left a comment
Contributor
There was a problem hiding this comment.
I won't be able to test directly but I looked at the modest code changes and everything looks correct to me. We should see significant load time improvement on large files with many streams.
Contributor
Author
|
I think it depends on whether this will result in significant performance gains (file size, size of the loaded streams, compressed or not, file on network), but it's a conceptual improvement for sure. |
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.
Fixes #60.
When
select_streamswas a list of dicts,load_xdffirst read the whole file withresolve_streams()to find the matching stream IDs, then read it again to load them. Now each StreamHeader is checked against the criteria while loading, and chunks from streams that don't match are skipped. Matching still usesmatch_streaminfoson the same stream-info dicts, now built by a shared_streaminfo()helper, so selection behaves as before. The "No matching streams found" error is now raised after reading the file instead of before. On a 478 MB test file, loading only a small selected stream became about twice as fast, because there is no longer a separate pass over the file.The new tests cover int, list-of-int and dict selections, and the no-match and invalid-input errors. They check that the file is opened only once and that the selected streams match a full load.