Issue 142 in pydicom: DA, DT, TM converters

[pyd...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium Difficulty-Medium

New issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[pyd...@googlecode.com]

Comment #1 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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.

[pyd...@googlecode.com]

Comment #2 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

Significant additional dependencies:
re
dateutil.tz

[pyd...@googlecode.com]

Comment #3 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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?

[pyd...@googlecode.com]

Comment #4 on issue 142 by darcy...@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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.

[pyd...@googlecode.com]

Comment #5 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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?

[pyd...@googlecode.com]

Comment #6 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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?

[pyd...@googlecode.com]

Comment #7 on issue 142 by Dimitri....@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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.

[pyd...@googlecode.com]

Comment #8 on issue 142 by darcy...@gmail.com: DA, DT, TM converters
http://code.google.com/p/pydicom/issues/detail?id=142

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.

[pyd...@googlecode.com]

Comment #9 on issue 142 by agrothberg: DA, DT, TM converters
https://code.google.com/p/pydicom/issues/detail?id=142

What ever came of this patch / addition?