-
Notifications
You must be signed in to change notification settings - Fork 262
WIP: Migrate load, save, class_map, ext_map to class properties and methods #329
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
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
200bac0
RF: Begin refactoring load into image classes
effigies 2cb5c95
Deprecate the use of class_map and ext_map.
f3373ca
Migrating imageclasses, away from IMAGE_MAP
45f50e3
Improve error handling, efficiency, and search more broadly over head…
20ad347
Modify save, remove vestigates of class_map / ext_map
91202cf
Code cleanup after self code review, fix for Python 3 'filter' issue.
6f40ad8
Remove references to BinOpener
7635ed2
Linting, cleaning up logic.
fab693e
Adding tests for loading each image type.
887423f
Add back analyze header tests.
e2a6fd8
sniff_size => sizeof_hdr
17b8253
Simplify is_image test.
4eec23d
Relegate slicing to is_header, remove _minctest
effigies 64e9ac1
TST: Remove Minc1Header exception
effigies 4d77eb4
STY: Unnecessary try block, comment lines too long
effigies afc9c25
TST: Test ValueErrors raised by is_header
effigies 1971efb
RF: Rename is_header to may_contain_header
effigies 4ddcbd2
RF: Restore and deprecate loadsave helpers
effigies 5570d36
TST: Kill extra test; doc string to comment
effigies 01af20f
RF: Prefer alternate_exts over abusing files_types
effigies 0c06464
RF: refactor using valid_exts, _meta_sniff_len
matthew-brett fce4cad
RF: rename compressed_exts to compressed_suffxes
matthew-brett 2ad5621
BF: isolate ImageOpener extension list
matthew-brett 5600d42
DOC: add docstrings for image loading classmethods
matthew-brett d8f1be9
RF: go back to using decorator for .mgz extension
matthew-brett ecee561
DOC: Deprecation warnings to stacklevel 2
effigies c3518d1
STY: Remove misleading 'sizeof_hdr' from Minc1/2
effigies 5814435
RF: Tag sniffs with file name
effigies 76440f2
RF: Fail quietly on too-short header blocks
effigies 8a926a4
TEST: path_maybe_image part of image API
effigies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,18 +10,12 @@ | |
""" Utilities to load and save image objects """ | ||
|
||
import numpy as np | ||
import warnings | ||
|
||
from .filename_parser import types_filenames, splitext_addext | ||
from .filename_parser import splitext_addext | ||
from .openers import ImageOpener | ||
from .analyze import AnalyzeImage | ||
from .spm2analyze import Spm2AnalyzeImage | ||
from .nifti1 import Nifti1Image, Nifti1Pair, header_dtype as ni1_hdr_dtype | ||
from .nifti2 import Nifti2Image, Nifti2Pair | ||
from .minc1 import Minc1Image | ||
from .minc2 import Minc2Image | ||
from .freesurfer import MGHImage | ||
from .spatialimages import ImageFileError | ||
from .imageclasses import class_map, ext_map | ||
from .imageclasses import all_image_classes | ||
from .arrayproxy import is_proxy | ||
|
||
|
||
|
@@ -40,9 +34,17 @@ def load(filename, **kwargs): | |
img : ``SpatialImage`` | ||
Image of guessed type | ||
''' | ||
return guessed_image_type(filename).from_filename(filename, **kwargs) | ||
sniff = None | ||
for image_klass in all_image_classes: | ||
is_valid, sniff = image_klass.path_maybe_image(filename, sniff) | ||
if is_valid: | ||
return image_klass.from_filename(filename, **kwargs) | ||
|
||
raise ImageFileError('Cannot work out file type of "%s"' % | ||
filename) | ||
|
||
|
||
@np.deprecate | ||
def guessed_image_type(filename): | ||
""" Guess image type from file `filename` | ||
|
||
|
@@ -56,39 +58,16 @@ def guessed_image_type(filename): | |
image_class : class | ||
Class corresponding to guessed image type | ||
""" | ||
froot, ext, trailing = splitext_addext(filename, ('.gz', '.bz2')) | ||
lext = ext.lower() | ||
try: | ||
img_type = ext_map[lext] | ||
except KeyError: | ||
raise ImageFileError('Cannot work out file type of "%s"' % | ||
filename) | ||
if lext in ('.mgh', '.mgz', '.par'): | ||
klass = class_map[img_type]['class'] | ||
elif lext == '.mnc': | ||
# Look for HDF5 signature for MINC2 | ||
# https://www.hdfgroup.org/HDF5/doc/H5.format.html | ||
with ImageOpener(filename) as fobj: | ||
signature = fobj.read(4) | ||
klass = Minc2Image if signature == b'\211HDF' else Minc1Image | ||
elif lext == '.nii': | ||
with ImageOpener(filename) as fobj: | ||
binaryblock = fobj.read(348) | ||
ft = which_analyze_type(binaryblock) | ||
klass = Nifti2Image if ft == 'nifti2' else Nifti1Image | ||
else: # might be nifti 1 or 2 pair or analyze of some sort | ||
files_types = (('image', '.img'), ('header', '.hdr')) | ||
filenames = types_filenames(filename, files_types) | ||
with ImageOpener(filenames['header']) as fobj: | ||
binaryblock = fobj.read(348) | ||
ft = which_analyze_type(binaryblock) | ||
if ft == 'nifti2': | ||
klass = Nifti2Pair | ||
elif ft == 'nifti1': | ||
klass = Nifti1Pair | ||
else: | ||
klass = Spm2AnalyzeImage | ||
return klass | ||
warnings.warn('guessed_image_type is deprecated', DeprecationWarning, | ||
stacklevel=2) | ||
sniff = None | ||
for image_klass in all_image_classes: | ||
is_valid, sniff = image_klass.path_maybe_image(filename, sniff) | ||
if is_valid: | ||
return image_klass | ||
|
||
raise ImageFileError('Cannot work out file type of "%s"' % | ||
filename) | ||
|
||
|
||
def save(img, filename): | ||
|
@@ -105,25 +84,38 @@ def save(img, filename): | |
------- | ||
None | ||
''' | ||
|
||
# Save the type as expected | ||
try: | ||
img.to_filename(filename) | ||
except ImageFileError: | ||
pass | ||
else: | ||
return | ||
|
||
# Be nice to users by making common implicit conversions | ||
froot, ext, trailing = splitext_addext(filename, ('.gz', '.bz2')) | ||
lext = ext.lower() | ||
|
||
# Special-case Nifti singles and Pairs | ||
if type(img) == Nifti1Image and ext in ('.img', '.hdr'): | ||
# Inline imports, as this module really shouldn't reference any image type | ||
from .nifti1 import Nifti1Image, Nifti1Pair | ||
from .nifti2 import Nifti2Image, Nifti2Pair | ||
if type(img) == Nifti1Image and lext in ('.img', '.hdr'): | ||
klass = Nifti1Pair | ||
elif type(img) == Nifti2Image and ext in ('.img', '.hdr'): | ||
elif type(img) == Nifti2Image and lext in ('.img', '.hdr'): | ||
klass = Nifti2Pair | ||
elif type(img) == Nifti1Pair and ext == '.nii': | ||
elif type(img) == Nifti1Pair and lext == '.nii': | ||
klass = Nifti1Image | ||
elif type(img) == Nifti2Pair and ext == '.nii': | ||
elif type(img) == Nifti2Pair and lext == '.nii': | ||
klass = Nifti2Image | ||
else: | ||
img_type = ext_map[ext] | ||
klass = class_map[img_type]['class'] | ||
else: # arbitrary conversion | ||
valid_klasses = [klass for klass in all_image_classes | ||
if ext in klass.valid_exts] | ||
if not valid_klasses: # if list is empty | ||
raise ImageFileError('Cannot work out file type of "%s"' % | ||
filename) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll go for the second, since we want a list anyway... |
||
klass = valid_klasses[0] | ||
converted = klass.from_image(img) | ||
converted.to_filename(filename) | ||
|
||
|
@@ -214,6 +206,7 @@ def read_img_data(img, prefer='scaled'): | |
return hdr.raw_data_from_fileobj(fileobj) | ||
|
||
|
||
@np.deprecate | ||
def which_analyze_type(binaryblock): | ||
""" Is `binaryblock` from NIfTI1, NIfTI2 or Analyze header? | ||
|
||
|
@@ -241,13 +234,16 @@ def which_analyze_type(binaryblock): | |
* if ``sizeof_hdr`` is 348 or byteswapped 348 assume Analyze | ||
* Return None | ||
""" | ||
hdr = np.ndarray(shape=(), dtype=ni1_hdr_dtype, buffer=binaryblock) | ||
bs_hdr = hdr.byteswap() | ||
sizeof_hdr = hdr['sizeof_hdr'] | ||
bs_sizeof_hdr = bs_hdr['sizeof_hdr'] | ||
warnings.warn('which_analyze_type is deprecated', DeprecationWarning, | ||
stacklevel=2) | ||
from .nifti1 import header_dtype | ||
hdr_struct = np.ndarray(shape=(), dtype=header_dtype, buffer=binaryblock) | ||
bs_hdr_struct = hdr_struct.byteswap() | ||
sizeof_hdr = hdr_struct['sizeof_hdr'] | ||
bs_sizeof_hdr = bs_hdr_struct['sizeof_hdr'] | ||
if 540 in (sizeof_hdr, bs_sizeof_hdr): | ||
return 'nifti2' | ||
if hdr['magic'] in (b'ni1', b'n+1'): | ||
if hdr_struct['magic'] in (b'ni1', b'n+1'): | ||
return 'nifti1' | ||
if 348 in (sizeof_hdr, bs_sizeof_hdr): | ||
return 'analyze' | ||
|
Oops, something went wrong.
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.
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.
Can we leave a stub here? I guess someone may have used this function, given it's got a public API name.
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.
Do you think a deprecation warning or return an image class like before, but using the new mechanism? (Or both?)
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.
Both, I think. Or do you think someone would want this function independent of the
load
mechanism?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.
Doesn't seem obvious that they would. If you say deprecate, that works for me.
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.
Could just be a stub that tries
load
and return the class of the result.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.
load
is only 7 lines. My inclination is to just replicate it without the.from_filename
bit. If we weren't deprecating, I'd haveload
call it like before, but might as well make the process of removing it easy.