BF+TST: Skip over missing CSA header elem instead of raising. #393
Conversation
|
The failure on 2.6 is from this pydicom bug which is now fixed in master. Should I mark it as a known failure? |
|
Marking as knownfailure just for Python 2.6 seems good. We can drop Python 2.6 soon, if it makes it easier, we could drop it now? |
| # Assume tag exists | ||
| tag = dcm_data[(0x29, element_no)] | ||
| try: | ||
| tag = dcm_data[(0x29, element_no)] |
There was a problem hiding this comment.
@moloney Can you add a comment about when this would fail, and why None is an OK value to return?
There was a problem hiding this comment.
I added a note about why it may fail. The docstring includes info about returning None.
|
Brendan - can I help with this one? |
|
Brendan - what news? Do you have time to take a look? |
Allows us to load data where the PrivateCreator is present but the element containing the header info is not. Ran into this with anonymized data.
Include a comment about why the tag might be missing.
Move pydicom import back to top level of module since we need its version info for skipping the test, plus this removes duplicate logic for importing 'pydicom' vs 'dicom'. Updated test_csa_header_read to use the top level import as well.
|
Great - thanks for finishing this one up. Ben - any more comments before I merge? |
|
Thanks - Brendan - merging as is, Ben feel free to follow up here with comments if you have them, we'll address them with a new PR. |
BF+TST: Skip over missing CSA header elem instead of raising. Allows us to load data where the PrivateCreator is present but the element containing the header info is not. Ran into this with anonymized data.
BF+TST: Skip over missing CSA header elem instead of raising. Allows us to load data where the PrivateCreator is present but the element containing the header info is not. Ran into this with anonymized data.
Allows us to load data where the PrivateCreator is present but
the element containing the header info is not. Ran into this with
anonymized data.