WIP: adding Philips XML/REC support #683
Open
Conversation
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
Philips now encourages the XML-style headers over the older PAR format. This initial implementation reads the XML file, converting each field to its PAR file equivalent and then inherits from the PARRECHeader and PARRECImage to avoid excessive code duplication.
…ecognized enum strings. add an error message with a hint regarding truncated XML files
Add an xmlrec2nii command line script. This currently is identical to parrec2nii because the underlying function now supports both formats.
prior to this commit, both would get stored as true because bool('N') == True
effigies
reviewed
Feb 3, 2019
| % bitpix) | ||
| # REC data always little endian | ||
| dt = np.dtype('uint' + str(bitpix)).newbyteorder('<') | ||
| super(PARRECHeader, self).__init__(data_dtype=dt, |
There was a problem hiding this comment.
This should be super(XMLRECHeader, ...
|
|
||
| @classmethod | ||
| def filespec_to_file_map(klass, filespec): | ||
| file_map = super(PARRECImage, klass).filespec_to_file_map(filespec) |
| @@ -46,6 +47,15 @@ def load(filename, **kwargs): | |||
| for image_klass in all_image_classes: | |||
| is_valid, sniff = image_klass.path_maybe_image(filename, sniff) | |||
| if is_valid: | |||
| if image_klass is PARRECImage and '.REC' in filename: | |||
There was a problem hiding this comment.
Suggested change
| if image_klass is PARRECImage and '.REC' in filename: | |
| if image_klass is PARRECImage and filename.endswith('.REC'): |
| @@ -19,6 +19,7 @@ | |||
| from .arrayproxy import is_proxy | |||
| from .py3k import FileNotFoundError | |||
| from .deprecated import deprecate_with_version | |||
| from .parrec import PARRECImage | |||
There was a problem hiding this comment.
Can you import this inside load? See the comment involved in importing Nifti* in save():
Lines 109 to 112 in d5494f3
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.
This is a PR based on the discussion in #681. It is still a work in progress.
I have now refactored the code so that there is no conversion from XML header names to PAR names. The native XML names are all retained. I think this makes it so that @Roosted7 can work from this PR.
Due to the large refactor to remove the PAR conversion, it does not make sense to review the PR starting from the first commit. I should probably rebase and squash some of that history.
@Roosted7. Does this version look suitable for you to work from? Feel free to modify or replace
parse_XML_header. As long as you provide thegeneral_infoandimage_defsin the same format so we can keep the same API as in the PARREC classes. New functions/methods can be added for writing an XML file. I don't intend to work on that part myself, but hopefully you can work from what I have started here instead of starting from scratch.Other things to potentially look into:
1.) I currently subclassed
PARRECHeaderandPARRECImage, but many methods fromPARRECHeaderhad to be overridden. Sometimes these were just to changeimage_defsfield names to the XML conventions. I suppose this could be refactored somewhat to use a common base class instead.2.) The XML parsing could/should probably reuse things from
xml_utils.pyObviously tests are still needed. @Roosted7 had linked to some PAR/XML/REC data in #681 and I can potentially provide some small example XML files from phantom scans.