- Notifications
You must be signed in to change notification settings - Fork269
WIP: adding Philips XML/REC support#683
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Philips now encourages the XML-style headers over the older PARformat. This initial implementation reads the XML file, convertingeach field to its PAR file equivalent and then inherits from thePARRECHeader 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 underlyingfunction now supports both formats.
prior to this commit, both would get stored as true because bool('N') == TrueThere 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.
Hi@grlee77. I found a few review comments sitting here. Not sure whether I was waiting on something or just forgot to hit "Submit".
Let me know when you're ready for a thorough review.
| %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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This should besuper(XMLRECHeader, ...
| @classmethod | ||
| deffilespec_to_file_map(klass,filespec): | ||
| file_map=super(PARRECImage,klass).filespec_to_file_map(filespec) |
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.
super(XMLRECImage, ...
| forimage_klassinall_image_classes: | ||
| is_valid,sniff=image_klass.path_maybe_image(filename,sniff) | ||
| ifis_valid: | ||
| ifimage_klassisPARRECImageand'.REC'infilename: |
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.
| ifimage_klassisPARRECImageand'.REC'infilename: | |
| ifimage_klassisPARRECImageandfilename.endswith('.REC'): |
| from .arrayproxyimportis_proxy | ||
| from .py3kimportFileNotFoundError | ||
| from .deprecatedimportdeprecate_with_version | ||
| from .parrecimportPARRECImage |
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 you import this insideload? See the comment involved in importingNifti* insave():
Lines 109 to 112 ind5494f3
| # Special-case Nifti singles and Pairs | |
| # Inline imports, as this module really shouldn't reference any image type | |
| from .nifti1importNifti1Image,Nifti1Pair | |
| from .nifti2importNifti2Image,Nifti2Pair |
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.