Skeleton object hierarchy and API definition (skeleton_object_hierarchy.py)

[suever]

From DaniN...@gmail.com on December 09, 2009 10:19:36

Quick hack of a condensed object hierarchy to concretize and substantiate aspects of the discussion that Darcy started with the "Proposal for reorganization of the DataElement types".

As discussed on the mailing lists I already had some ideas regarding the object hierarchy, most of which are concisely exemplified in this file.

The most important part of this file supposedly is the doctests in the function "test_skeleton_implementation()", which could serve as an API- specification, whose functionality, possibly performance, etc. could be tested in a straightforward manner. Just run the script as "main" to test the doctests.

I currently believe that the approach sketched here

1) is or could be made backwards compatible with pydicom 2) concentrates all the "magic" of multiple-style acces on a "custom" top- level object (DicomPart10File) 3) allows flexible treatment of DataElement values, whether multi-valued or not (?) 4) seems to allow all styles of attribute access that Darcy asked for (?) without much additional restructuring and or a need to have DataElements subclass a variety of builtins (?). 5) does not attempt to stick parts of a dicom file (preamble) onto other parts of the file (dataset)

It is well possible that I am missing lots of blatantly obvious things, so feel free to just ignore this :-)

DaniN

Attachment: skeleton_object_hierarchy.py

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

[suever]

From darcymason@gmail.com on December 13, 2009 11:25:39

Dani, I really appreciate your efforts on this... and I haven't had a proper chance to review your code yet. But at the same time, I wanted to continue the discussion by including the code I had previously mentioned I was working on. A start on that has been pushed up to another repository clone at https://code.google.com/p/pydicom/source/clones . It handles the VR re-working, but not your comments on Part10 objects.

I've set that repository so anyone can do code reviews, so you could add comments there or continue discussing here, whichever makes the most sense for the types of comments, I guess.

Between your efforts and mine (and any others who wish to add thoughts), hopefully we can come to a consensus and begin making the code changes.

Regards, Darcy

[suever]

From darcymason@gmail.com on December 13, 2009 16:05:04

I realized I should have added some info in my comment above about the new repository to point people to the experimental code. The relevant code is in a new subdirectory called 'vr' under the dicom directory. Also of interest are the test_DS and test_IS unit tests in the test directory (and I'm working on test_multival which may go up soon). These files are disconnected from use in pydicom datasets so far. They would have to be hooked into the Dataset.setitem, setattr etc. so that any normal python types passed in could be coerced to the proper type for the VR of the tag being set.

The ideas morphed quite a bit while I was working on them -- in the end, I wound up with the VR's as new types in python, where validation is done right when the object is created (in new). I had thought about leaving validation until the end, but realized that if someone wanted to write code with non-valid DICOM values, they could do so with regular python types outside the Dataset, and only add it to the dataset at the end (by then it had better be a valid DICOM value anyway).

[suever]

From darcymason@gmail.com on December 25, 2009 20:53:59

I decided to use DaniN's skeleton API concept before finishing my changes in the experimental repository -- attached is a single file which completes my ideas for VR types, showing everything in one place (only the new VR type concepts, not the Part10File ideas). I didn't use exactly the same function names, etc. as DaniN's, because of trying to match the existing pydicom names (or what they should be with PEP-8 style) as closely as possible.

The main section at the bottom has tests to verify the correct behaviour. Single values and multi-value elements are both available, with DICOM validity checking (which can be turned off if desired).

With this concept, there would be no DataElement objects. The Dataset simply stores values derived from VR_base. Since the current pydicom DataElement stores VR, VM, tag as well as value, then for backwards compatibility, these new VR types would support VR, VM and value attributes. VM is not done for the MultiValue class in this example, but can be trivially added (with len(self)). I think 'value.tag' should not be used because a "value" should be pure -- the same one could be bound to multiple items having different tags.

I don't think there is a lot of "magic" in this code, and it actually simplifies some things by removing the DataElement class completely. The DICOM dataset is represented as a python dict of tag: value pairs, where the values are essentially python types, but are further restricted according to DICOM rules.

Unless anyone can point out flaws in these concepts, my plan would be to make this change (including classes for all VR types) for pydicom 0.9.5 (after version 0.9.4 -- the improvements to file reading, done in the main repository).

-Darcy

Status: Accepted
Labels: -Priority-Medium Priority-High Milestone-NextRelease

Attachment: skeleton_vr.py

[suever]

From darcymason@gmail.com on December 15, 2011 17:56:49

Changing priority on this, not expecting to revisit it in the short term. The current data conversion is working well, except that Decimal could be improved ( issue 43 ).

Labels: -Type-Defect -Priority-High -Milestone-NextRelease Type-Enhancement Priority-Low Milestone-Release1.5