ENH: Added DICOMUtils convenience function library #421
Conversation
| # Get patient UID by patient name for easy loading of a patient | ||
| def patientUIDByPatientName(name): | ||
| if not hasattr(slicer,'dicomDatabase'): | ||
| return None |
There was a problem hiding this comment.
Should an error or warning be logged ?
|
I added a DICOM utility library to make it easier to do DICOM DB operations from python.
|
c520812
to
048db2c
Compare
| def openDatabase(databaseDir): | ||
| if not hasattr(slicer,'dicomDatabase') or not hasattr(slicer.modules,'dicom'): | ||
| logging.error('DICOM module or database cannot be accessed!') | ||
| return |
There was a problem hiding this comment.
It would be useful to return with a different value depending on if the database could be successfully opened or not
There was a problem hiding this comment.
Thanks! I fixed it before saw your comment :)
There was a problem hiding this comment.
Oh I see. So it should check if opening was successful?
There was a problem hiding this comment.
Yes, it would be nice if you could check if the opening was successfull, too.
048db2c
to
06fddae
Compare
|
|
||
| #------------------------------------------------------------------------------ | ||
| # Get patient UID by patient name for easy loading of a patient | ||
| def patientUIDByPatientName(name): |
There was a problem hiding this comment.
This method name is not very clear for me. getPatientUIDByPatientName or patientUIDFromPatientName would be more clear.
There was a problem hiding this comment.
A small improvement possibility (probably not important for tests but could be useful for general use): In DICOM usually the patient name and patient ID are used together to identify a patient. It may be useful to add an optional patientID parameter that would allow to search for a specific patient name AND id.
There was a problem hiding this comment.
I will rename the function.
This file will be open to additional methods for all developers if the need arises.
There was a problem hiding this comment.
Also, "patientUID" should be replaced with "patientID". There is no such thing as "patient UID" (unfortunately!).
|
Thanks Csaba, this will be useful. Last time when somebody wanted to merge utility python functions I insisted that the utilities should be used by the Slicer core modules wherever it is applicable to serve as an example and to not duplicate code (see Slicer/Slicer#407). So, I'm afraid I would need to ask you the same: please update the JRC2013Vis, RSNAVisTutorial and SubjectHierarchyGenericSelfTest tests as well. |
|
I didn't do that in the hope of quick merge, as I don't think touching the RSNA tutorial is wise right now. |
|
RSNA demo will use stable and nothing will be merged until layer this week, when a new release will be created for additional RSNA activities. So, there will be no impact on RSNA. Using the utility functions in the tests will also serves as tests for these new utility functions (in addition to serve as examples and reduce code duplication). |
06fddae
to
4d02bf7
Compare
|
I changed the python tests using temporary databases to call the new utility functions |
Usage examples:
from DICOMLib import DICOMUtils
with DICOMUtils.TemporaryDICOMDatabase(folderName):
[ Code here to import SamplePatient from local storage ]
DICOMUtils.loadPatientByName('SamplePatient')
4d02bf7
to
8b6a133
Compare
|
I added the context manager for temporary databases. |
|
@cpinter After return the database in the context manager. LGTM 👍 |
|
Did the requested change. |
Usage:
import DICOMLib
DICOMLib.DICOMUtils.loadPatientByName('SamplePatient')