Hotfix/52 empty referring physician and operators_name in exported dicom images#53
Conversation
…ringPhisicianName, OperatorName, and ScheduledPerformingPhysicianName
zgypa
left a comment
zgypa
left a comment
There was a problem hiding this comment.
- It doesn't look like you validated the generated DICOM with a validator, otherwise you should have gotten errors about not allowed tags in the IOD.
- The PR looks rushed: it seems like you missed a specific place for the code you wrote, and you put it somewhere more generic, without explaining why.
- I'm available if you need help with understanding the issue you are trying to address. I commented the issue with a question.
|
|
||
| # RequestingPhysician, ReferringPhysicianName, OperatorsName, ScheduledPerformingPhysicianName | ||
| if 'RequestingPhysician' in self.dicom_mwl: | ||
| self._ds.RequestingPhysician = self.dicom_mwl.RequestingPhysician |
There was a problem hiding this comment.
Non-valid DICOM tag in VL IOD, or any IOD at all. I think it's a Modality Worklist specific tag.
| self._ds.ReferringPhysicianName = self.dicom_mwl.ReferringPhysicianName | ||
|
|
||
| if 'OperatorsName' in self.dicom_mwl: | ||
| self._ds.OperatorsName = self.dicom_mwl.OperatorsName |
There was a problem hiding this comment.
I don´t think you can have an (0008,1070) OperatorsName in a DICOM Modality Worklist. This is information that only the modality knows, while the Modality Worklist is generated usually the the RIS or PMS, and has no idea who will perform the acquisition.
| ev20.CodeMeaning = "Extraoral, Full Face, Full Smile, Centric Relation" | ||
| sps.ScheduledProtocolCodeSequence.append(ev20) | ||
|
|
||
| # Opeators name |
There was a problem hiding this comment.
See above: OperatorsName not allowed or known in MWL.
| o.copy_mwl_tags(dicom_mwl=mwl) | ||
|
|
||
| # Test the testOperatorName method | ||
| self.assertEqual(o._ds.OperatorsName, mwl.OperatorsName) |
There was a problem hiding this comment.
mwl.OperatorsName should not exist.
|
|
||
| if 'ScheduledProcedureStepSequence' in self.dicom_mwl and len(self.dicom_mwl.ScheduledProcedureStepSequence) > 0: | ||
| scheduledPerformingPhysicianName = self.dicom_mwl.ScheduledProcedureStepSequence[0].ScheduledPerformingPhysicianName | ||
| for step in self._ds.RequestAttributesSequence: |
There was a problem hiding this comment.
Document the reason to add an attribute in the RequestedAttributeSequence here, instead of in _set_request_attributes() written purposely to do this task.
Why did you call them "step"? Took me a while to figure out this is probably just a poor name choice. You are looping through a sequence of request attributes. So calling this request_attribute or just attribute, would make the life of the code reader easier.
| if 'ScheduledProcedureStepSequence' in self.dicom_mwl and len(self.dicom_mwl.ScheduledProcedureStepSequence) > 0: | ||
| scheduledPerformingPhysicianName = self.dicom_mwl.ScheduledProcedureStepSequence[0].ScheduledPerformingPhysicianName | ||
| for step in self._ds.RequestAttributesSequence: | ||
| step.ScheduledPerformingPhysicianName = scheduledPerformingPhysicianName |
There was a problem hiding this comment.
0040,0006 Scheduled Performing Physician's Name is not part of any IOD, that i was able to find.
|
|
||
| scheduledPerformingPhysicianName = mwl.ScheduledProcedureStepSequence[0].ScheduledPerformingPhysicianName | ||
| for step in o._ds.RequestAttributesSequence: | ||
| self.assertEqual(step.ScheduledPerformingPhysicianName, scheduledPerformingPhysicianName) |
2 participants
No description provided.