Page MenuHomePhabricator

Cannot compare TemporoSpatialStringProperty
Closed, WontfixPublic

Description

In this scenario a DICOM image is loaded and stored in the data storage. In order to find this data node later, the function mitk::DataNode::Pointer node = m_DataStorage->GetNode(...) is used.
As a parameter a NodePredicateDataProperty has been used that is initialized with a TemporoSpatialStringProperty:

std::string propertyName = GeneratePropertyNameForDICOMTag(0x0020, 0x000E);
const std::string& DICOMID = "TheDICOMValueOfTheDICOMTag";
TemporoSpatialStringProperty::Pointer DICOMIDProperty = TemporoSpatialStringProperty::New(DICOMID);
NodePredicateDataProperty::Pointer DICOMIDPredicate = NodePredicateDataProperty::New(propertyName.c_str(), DICOMIDProperty);

return DICOMIDPredicate;

When usind the GetNode(...)-function, the NodePredicateDataProperty::CheckNode-function is called. Here the TemporoSpatialStringProperty of the node predicate is compared to the TemporoSpatialStringProperty of the data node.
Here, the properties never match: The created TemporoSpatialStringProperty has only one entry (the DICOM Tag with it's value), but the original data node has e.g. 400 entries, all with the same values. I guess this is due to the number of 400 dicom images in the folder that represent the DICOM image.

How is this property to be used?

Event Timeline

kalali triaged this task as Normal priority.Jul 27 2018, 6:37 PM
kalali created this task.
kalali renamed this task from Cannot compare TemporoSpatialStringPropertys to Cannot compare TemporoSpatialStringProperty.Aug 14 2018, 9:20 AM
kalali added a subscriber: hentsch.

You pointed in the right direction. The operator == currently checks complete equality. So also the number of time points and frames must be the same; and must have the same value. This is not the case in your on-the-fly-generated operand. Tempering with the == is not trival, because there are use cases where it realy matters if there is the same geometry and not only the same value. There are thoughts to implement posibilities to express TemporoSpatialStringProperties more compact, but thats no quick solution.

In your case I would propose the following sound workarround/alternative. Instead of using NodePredicateDataProperty use NodePredicateFunction.
Pass it a lambda that compares the GetValueAsString() result of the properties for equality. That should do the trick, because then just the value and not the geometry matters.

Thanks, I will give it a try.

kislinsk added a project: Auto-closed.
kislinsk added a subscriber: kislinsk.

Hi there! ๐Ÿ™‚

This task was auto-closed according to our Task Lifecycle Management.
Please follow this link for more information and don't forget that you are encouraged to reasonable re-open tasks to revive them. ๐Ÿš‘

Best wishes,
The MITK devs

I vaguely remember what it was used for: In the context of SemanticRelations but I can't find a related commit. So I guess we just leave it as it is and see if it pops up (once we talk again about the SemanticRelations and its future use).

floca removed a project: Auto-closed.
floca added a subscriber: โ€ข thomass.

@thomass This is the issue we talked about last week when you hat problem to use predicates to find some nodes by DICOM tags values.

I think we should somehow handle it to provide a documented way for that. The alternative would be that we have more developers to come that stumple upon the subtility of TemperoSpatialStringProperty comperison.

I see to pragmatic options:

  1. Add to NodePredicateDataProperty an option to only compare by GetValueAsString.
  2. Add a specific NodePredicateTemperoSpatialStringProperty that also offers the option that you only compare the value(s) but don't care about number of slices or timesteps.

#1 is basically the workaround we already use with the NodePredicatFunction. And it is more generic as it also could be used with other properties.
#2 would be a bit more explizit that something special is about the TemperoSpatialStringProperties and who to use predicated on them.

What do you think?

Since solution 1 worked well for me and seems to be more generic, I would vote for 1.

kislinsk added a project: Auto-closed.

Hi there! ๐Ÿ™‚

This task was auto-closed according to our Task Lifecycle Management.
Please follow this link for more information and don't forget that you are encouraged to reasonable re-open tasks to revive them. ๐Ÿš‘

Best wishes,
The MITK devs