Skip to content

ENH: Add to/from_stream methods and from_url classmethod to SerializableImage #1129

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

Merged
merged 8 commits into from
Aug 31, 2022

Conversation

effigies
Copy link
Member

Extends #639 to allow from_url() to load an image. Does not currently handle gzipped (see #1128 for some more discussion) streams, though this should be doable.

Abstracts to using io.IOBase to permit easy extension to other streams, but does not expose it generically at this point.

Tested to work on https://openneuro.org/crn/datasets/ds000117/snapshots/1.0.5/files/sub-01:ses-mri:fmap:sub-01_ses-mri_magnitude1.nii and a file:///... option.

@codecov
Copy link

codecov bot commented Aug 15, 2022

Codecov Report

Merging #1129 (82c50ba) into master (b38a99b) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1129      +/-   ##
==========================================
+ Coverage   91.74%   91.78%   +0.03%     
==========================================
  Files         101      101              
  Lines       12382    12392      +10     
  Branches     2424     2424              
==========================================
+ Hits        11360    11374      +14     
+ Misses        694      692       -2     
+ Partials      328      326       -2     
Impacted Files Coverage Δ
nibabel/filebasedimages.py 92.05% <100.00%> (+3.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@effigies
Copy link
Member Author

Anybody interested in reviewing?

@matthew-brett
Copy link
Member

Sorry - yes - will do - tomorrow.

Copy link
Member

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we automatically recognize the URL and plumb this into nib.load etc?

Copy link
Member

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops - sorry - forgot to add this comment.

@effigies
Copy link
Member Author

Can we automatically recognize the URL and plumb this into nib.load etc?

Probably. Feels like a bit of a can of worms that I didn't want to open in this PR. Okay to push off to another one?

@effigies effigies changed the title ENH: Add from_url classmethod to SerializableImage ENH: Add to/from_stream methods and from_url classmethod to SerializableImage Aug 30, 2022
@matthew-brett
Copy link
Member

Probably. Feels like a bit of a can of worms that I didn't want to open in this PR. Okay to push off to another one?

Yes, that's fine.

@matthew-brett
Copy link
Member

LGTM

@effigies effigies merged commit 5719019 into nipy:master Aug 31, 2022
@effigies effigies deleted the enh/streams branch August 31, 2022 13:25
@effigies
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants