Skip to content

MRG: some refactoring of PARREC API #264

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 6 commits into from
Oct 24, 2014

Conversation

matthew-brett
Copy link
Member

Add fake truncated file for testing, and test.
Add permit_truncated as header attribute.
Fix header copy to use permit_truncated.
Add file loading API methods with PARREC-specific parameters.

@matthew-brett
Copy link
Member Author

The file-loading API needs some thought, because I would also like to be able to pass in a parameter mmap to disable or enable memory mapping - to all the image classes. So maybe permit_truncated and scaling should be keyword only.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 04e953a on matthew-brett:parrec-fix-refactor into 7428ec4 on nipy:master.

@@ -82,9 +82,6 @@
from copy import deepcopy
import re

from .externals.six import string_types
from .py3k import asbytes

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI I already have these changes in my PR #263

@larsoner
Copy link
Contributor

Makes sense to have permit_truncated and scaling kwarg only for now. Alternatively, if you're sure you're going to add it later, you can put mmap=None in wherever you want it to eventually go, and just raise NotImplementedError for now if mmap is not None, and say in the docstring it's a placeholder for future compatibility.

Test that truncated PAR/REC file raises error or warning according to
`permit_truncated` flag.
Add `permit_truncated` to header attributes. Fix `copy` method to use
value of `permit_truncated` when making a copy. Test.
Add methods with optional args `permit_truncated` and `scaling`:

* from_file_map
* from_filename
* load
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling e20a1cd on matthew-brett:parrec-fix-refactor into 863d3d3 on nipy:master.

Add decorator to make keyword arguments keyword-only for Python 2.
Use and test keyword-onliness of permit_truncated, scaling argument to
PARRECImage file loading methods.
@matthew-brett
Copy link
Member Author

I added the keyword-only stuff - I think it's ready now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling 2040239 on matthew-brett:parrec-fix-refactor into 863d3d3 on nipy:master.

@larsoner
Copy link
Contributor

Fancy solution, I like it. LGTM.

matthew-brett added a commit that referenced this pull request Oct 24, 2014
MRG: some refactoring of PARREC API

Add fake truncated file for testing, and test.
Add permit_truncated as header attribute.
Fix header copy to use permit_truncated.
Add file loading API methods with PARREC-specific parameters.
Make PARREC-specific parameters keyword-only.
@matthew-brett matthew-brett merged commit 4d8a6c0 into nipy:master Oct 24, 2014
@matthew-brett matthew-brett deleted the parrec-fix-refactor branch October 24, 2014 23:10
grlee77 pushed a commit to grlee77/nibabel that referenced this pull request Mar 15, 2016
MRG: some refactoring of PARREC API

Add fake truncated file for testing, and test.
Add permit_truncated as header attribute.
Fix header copy to use permit_truncated.
Add file loading API methods with PARREC-specific parameters.
Make PARREC-specific parameters keyword-only.
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.

3 participants