-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add ReaderOptions and ReaderStreamedOptions #70
Conversation
- Make InMemNiftiObject's methods privates - Make StreamedNiftiObject's methods privates - Remove the deprecated new_from_stream method - Apply one fix on header when requested - Fix all tests
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.
Thank you for working on this @nilgoyette. Please see the comments, the most important concerns are (1) whether to keep former public APIs and (2) document the concrete header fixes.
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.
👍 Thanks!
Sorry, there was some automatic formatting when I saved (like in lib.rs). The other concerns I have at the moment are:
pub crate
inheader.rs
andvolume/inmen.rs
?ReaderOptions
andReaderStreamedOptions
to avoid repetitive namesread_file_streamed/_rank/_pair/_pair_rank
(I can at least remove the 'streamed' in all names. But it does create some code copy. If we ever add some general methods, likeread_header
, the code will be exactly the same. So, I'm not sure it was the right decision.