Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

33 DICOM metadata API #117

Merged
merged 9 commits into from
Sep 28, 2017
Merged

Conversation

rahit
Copy link
Contributor

@rahit rahit commented Sep 16, 2017

API implementation for dicom metadata and preview available at:
/api/images/metadata?dicom_location='IMAGE_PATH'.

Description

Before selecting particular DICOM image for analysis we need to display its metadata with preview. This API provides those data.

Reference to official issue

#33

How Has This Been Tested?

Test case to check HTTP 200 OK status for valid dicom image has been added. However, because of the issue #116, test cases are failing for the test images. I am adding screenshot which I verified using a dicom image from original dataset.

Screenshots (if appropriate):

image metadata api django rest framework

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

return s.replace(u"\u0000", "").strip()

def _convert_value(self, v):
t = type(v)
Copy link
Contributor

Choose a reason for hiding this comment

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

The typical way to do this in Python is using isinstance.

cv = repr(v)
return cv

def dicom_dataset_to_dict(self, ds):
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I think Django has some kind of idea about a serializer? Perhaps we should extend support via that instead of these custom routines? Alternatively, we could extend Django's JSON serializer? Anyway I think we should have this in utils.py or similar anyway :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Put dicom metadata into a separate dictionary
'''
dicom_dict = {}
repr(ds)
Copy link
Contributor

Choose a reason for hiding this comment

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

?

'1.3.6.1.4.1.14519.5.2.1.6279.6001.490157381160200744295382098329/' \
'1.3.6.1.4.1.14519.5.2.1.6279.6001.619372068417051974713149104919/-80.750000.dcm"'
})
self.assertEqual(response.status_code, status.HTTP_200_OK)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to test for some specific metadata attributes that should be correct and are known for the image? Like SeriesInstanceUID?

path = request.GET.get('dicom_location')
if path is None:
raise Exception('dicom_location not provided')
path = path[1:-1] # un-quoting the string
Copy link
Contributor

Choose a reason for hiding this comment

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

Hang on, why would we be quoted?

Copy link
Contributor

Choose a reason for hiding this comment

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

(Please clarify in docstring for posterity, not here....)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lamby unquoted path seems to create problem with existing url patterns.

Copy link
Contributor

Choose a reason for hiding this comment

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

"seems to"?

And again, please clarify the reason in the docstring itself, otherwise these important comments just get lost :)

'''
path = request.GET.get('dicom_location')
if path is None:
raise Exception('dicom_location not provided')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't recommend raising Exception - there is almost always a better one. Indeed, why not just do a raw request.GET and let it KeyError and — potentially — reraise it as a ValueError. I mean, that's going to be far more useful to the callsite.

@lamby
Copy link
Contributor

lamby commented Sep 27, 2017

@rahit As I understand it, this is waiting on you? :)

@rahit
Copy link
Contributor Author

rahit commented Sep 27, 2017

@lamby sorry for the late.

@lamby lamby merged commit 87df6fc into drivendataorg:master Sep 28, 2017
@lamby
Copy link
Contributor

lamby commented Sep 28, 2017

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants