Skip to content
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

DA, DT, TM converters #143

Closed
darcymason opened this issue Nov 27, 2014 · 21 comments
Closed

DA, DT, TM converters #143

darcymason opened this issue Nov 27, 2014 · 21 comments

Comments

@darcymason
Copy link
Member

From Dimitri....@gmail.com on April 18, 2014 20:11:27

As suggested in discussion "TM, DA converters?" tags DA, DT, TM could be converted to datetime objects: https://groups.google.com/forum/#!topic/pydicom/4IMwZiFgCkk

Original issue: http://code.google.com/p/pydicom/issues/detail?id=142

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on April 18, 2014 17:16:47

Please find a proof-of-concept here: https://code.google.com/r/dimitripapadopoulos-pydicom/source/detail?r=11dac0718ead057e40108337096c22f3c4d0b8a4 For now tested on a pair of DICOM files only. Just wanted some feedback before adding comments and testing more extensively.

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on April 18, 2014 17:17:56

Significant additional dependencies:
re
dateutil.tz

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on April 27, 2014 06:10:13

I have a new version with a dicom.config.datetime_conversion switch to toggle between previous behavior and conversion to datetime objects. https://code.google.com/r/dimitripapadopoulos-pydicom/source/detail?r=92da991a405fa8c9e16c6147ba32160eeaacfdcb Any additional changes?

@darcymason
Copy link
Member Author

From darcymason@gmail.com on April 28, 2014 18:42:57

Looks good. Adding unit tests would be much appreciated, for some normal values and for some corner cases. Also I didn't notice anything yet on the writing side -- the existing "round-trip" read/write unit tests (a couple of files maybe) could be copied to do tests with the config conversion on.

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on May 01, 2014 03:44:49

I have added unit tests and documentation: https://code.google.com/r/dimitripapadopoulos-pydicom/source/detail?r=7aa18b660782182d6e72564185f796abeae9ef52 Which are the "round-trip" read/write tests to run with the config conversion on?

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on May 01, 2014 06:16:16

And by the way, I must admit I'm not sure what needs to be added on the writing side. A standalone function to create a data element from a datetime object? But that's not in the filewriter code, is t?

@darcymason
Copy link
Member Author

From Dimitri....@gmail.com on May 02, 2014 01:19:49

I seldom use the part of pydicom that writes DICOM files. I've spent a few hours on it and I think I have understood enough of it:

  • to add a test by merely inheriting WriteFileTests,
  • to fix dataelem.py so that DA, DT, TM data elements are still created as strings if datetime_conversion is not set.

I'm not sure I'm doing the right thing when it comes to initializing data elements from datetime objects - see the new() method of DA, DT, TM classes.

@darcymason
Copy link
Member Author

From darcymason@gmail.com on May 07, 2014 06:37:47

Hi Dimitri,
I've had a look through the code and everything looks good. I'll pull it into my local clone and go through it in more detail.

@darcymason
Copy link
Member Author

From agrothberg on November 09, 2014 13:09:56

What ever came of this patch / addition?

@DimitriPapadopoulos
Copy link
Contributor

Would a Git branch forked from current dev branch help moving forward?

@darcymason
Copy link
Member Author

Yes that would be very helpful. Thanks.

@cancan101
Copy link
Contributor

Any chance of getting this merged for the v1 release?

@PhySci
Copy link

PhySci commented Feb 5, 2018

Hi everyone,
Does DA, DT, TM converter work in the current release?

@DimitriPapadopoulos
Copy link
Contributor

It should work, yes. Any specific issue?

@DimitriPapadopoulos
Copy link
Contributor

Note that you need to set config.datetime_conversion first.

@PhySci
Copy link

PhySci commented Feb 6, 2018

Many thanks for the reply.
I set datetime_conversion , read an MRI file:

import dicom
dicom.config.datetime_conversion = True

img = dicom.read_file('example.dcm')
img

and saw that DA and TM properties are still strings, like '20001106'.

Have I missed something?

@DimitriPapadopoulos
Copy link
Contributor

Are the DA and TM properties actual strings, or are they DA TM objects interpreted/printed as strings in a certain context?

@PhySci
Copy link

PhySci commented Feb 6, 2018

type(img.data_element('AcquisitionDate').value) returns 'str'

@DimitriPapadopoulos
Copy link
Contributor

Indeed, using the default pydicom on my account with Ubuntu 16.04 (with NeuroDebian):

>>> import dicom
>>> dicom.config.datetime_conversion = True
>>> dataset = dicom.read_file('example.dcm')
>>> dataset.AcquisitionDate
'20170101'
>>> type(dataset.AcquisitionDate)
<type 'str'>
>>> 

I would need to double-check which version I'm using, which one is provided by the system and/or NeuroDebian, etc. Unfortunately I won't have time in the short term. If anyone knows about it...

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Feb 10, 2018

The latest release is 0.9.9.post1.

The datetime_conversion mechanism should appear in release 1.0.0, whenever it is released.

@mrbean-bremen mrbean-bremen added this to the v1.0 milestone Feb 18, 2018
@mrbean-bremen
Copy link
Member

Shall be fixed now with the release.

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

No branches or pull requests

5 participants