Skip to content

increase sanity threshold for n_items per csa tag #242

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 4 commits into from
Jul 30, 2014

Conversation

kevlarkevin
Copy link
Contributor

This pull request increases the sanity threshold for the number of items per CSA Header tag from 100 to 200. Some derived Siemens dicoms have CSA tags with more than 100 items.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 55f3edd on kevlarkevin:increase_csa_nitems_assertion into 55daf7d on nipy:master.

@matthew-brett
Copy link
Member

Thanks for this - would you mind adding a test?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 2690953 on kevlarkevin:increase_csa_nitems_assertion into 55daf7d on nipy:master.

@matthew-brett
Copy link
Member

Thanks for that. I submitted a PR will a few small changes.

I noticed that when I increased the threshold for n_items, to try and read the 200n string, I got another error - is the 200n csa string valid?

Are you sure there is no identifiable patient data in the csa files?

@kevlarkevin
Copy link
Contributor Author

The previous 200n csa string was not valid. It has been replaced with a valid one (it worked for me). Both new csastrings have absolutely no identifiable patient data.

note: force pushed over 1 commit.

Make error more explicit for too many items in the csa structure.

Add module global for max items and test
@matthew-brett
Copy link
Member

Thanks for that. Consider also merging my PR to your branch? kevlarkevin#1

@kevlarkevin
Copy link
Contributor Author

Thanks for your PR. Much better test than what I had.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling eab18a3 on kevlarkevin:increase_csa_nitems_assertion into 27e4feb on nipy:master.

matthew-brett added a commit that referenced this pull request Jul 30, 2014
MRG: increase sanity threshold for n_items per csa tag

Allow more items in csa tags.
@matthew-brett matthew-brett merged commit 484fdd4 into nipy:master Jul 30, 2014
grlee77 pushed a commit to grlee77/nibabel that referenced this pull request Mar 15, 2016
…rtion

MRG: increase sanity threshold for n_items per csa tag

Allow more items in csa tags.
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