[Insight-users] LabelMap related issues

[Cagatay Bilgin]

Hi Gaëtan,

I have reported the 5th item as a bug, created a test, made a
proposed fix and the fix works on my system. I am reading about
how to submit fixes and will try do that in a couple of days. I will
also try to work on the rest of the items and add them to the issue
tracker, either as bugs or feature requests.

Thank you for this great module by the way!

Cagatay

On Fri, Feb 17, 2012 at 1:46 AM, Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr
> wrote:

>
> Hi Cagatay,
>
> Thanks for your very useful feedback.
> Please find my comments inlined in your original email.
>
> Le 16 févr. 12 à 18:51, cagatay bilgin a écrit :
>
>
>  I've been using LabelMap related filters and had a
>> few issues with them. These issues could stem from my
>> lack of knowledge about the design of these filters but I thought
>> it is good to have them on the list so that someone else could benefit
>> from or maybe some sort of cleanup can be performed on them.
>>
>> 1) LabelObject::SetLabel method has no
>> effect as the labels are stored in the LabelMap. If one
>> wants to change the labels, they need to go through the
>> ChangeLabelMapFilter or RelabelLabelMapFilter.
>>
>>
> Absolutely.
>
>
>  This function is just confusing as you can not properly
>> change the label, but it gives you the impression
>> that the label is changed. Either LabelObject::SetLabel
>> should do the expected behavior and change the label,
>> or this method should be dropped or renamed or return
>> something to warn you.
>>
>>
> It does what is expected if you are manipulating the label object outside
> of a LabelMap.
>
> But you're right that this can be confusing - could you propose an
> enhanced documentation?
>
>
>  2) Assuming that I have a labelobject that I created somehow...
>> When I want to add this labelobject to a labelmap, there
>> are two functions available.
>> LabelMap::AddLabelObject
>> LabelMap::PushLabelObject
>>
>> PushLabelObject searches for an unused label(largest label+1),
>> whereas AddLabelObject does not. If the label of the object at
>> hand clashes with another one in the labelmap, AddLabelObject
>> does not warn you, and just overwrites whatever was there.
>> PushLabelObject on the other hand looks for an unused label,
>> and changes the label of the object with whatever is available.
>>
>> It is nice to have these two different options but these should be
>> explicitly stated somewhere. I would also suggest to propagate
>> the return value (pair<iter,bool>) of the STL map's insert, instead
>> of having just void.
>>
>
> I think you're right.
> Could you propose a patch or file a bug report?
>
>
>
>> 3) All the features are accessed by GetProperty methods,
>> e.g. GetBoundingBox, GetPhysicalSize etc except
>> the size. It took me a while to realize that the function
>> was not GetSize but just Size. I would expect to have
>> GetSize instead of Size.
>>
>
> Size() is not a simple accessor - it computes the number of pixels in the
> label object each time.
> But you can use GetNumberOfPixels() if you want an accessor.
>
>
>
>> 4) There are multiple ways of converting the label map
>> to an image but as far as I can see no direct support of
>> converting "a" label object to its corresponding image
>>
>> i) I need to create a new empty label map,
>> ii) call copyInformation or SetRegions with the
>> bounding box of the object depending on the
>> type of image I am interested
>> iii) add the labelobject to the labelmap
>> iv) and then use labelmaptolabelimage filter.
>>
>> I guess a GetImage function could be provided.
>>
>>
> I don't think it can be easily done, because the LabelObject does not
> carry any information on the image, like its size, its position, its
> spacing, its orientation, ...
> And the GetImage() method should be template and we tend to avoid that.
>
>
>  5) If for some reason, possibly due to a not well thought design of mine,
>> I call BinaryImagetoShapeLabelMap with int type,
>> the InputforegroundValue is set to -1 and
>> the OutputBackgroundValue is set to 0 by the filter by default.
>> I would expect the default InputForegroundValue to be
>> +max int or 1 or 255, but not -1.
>>
>
> Really? This is definitely a bug then.
> Please file a bug report if you can confirm that behavior.


>
>
>> 6) In BinaryImagetoShapeLabelMap
>> there is SetInputForegroundValue to modify the foreground but
>> there is no SetInputBackgroundValue. Likewise, SetOutputBackgroundValue
>> exists, but SetOutputForegroundValue does not exists.
>> I would expect the missing functions to be available unless
>> there is good reason to not to.
>>
>
> I would say there wouldn't be any good reason to add them, because they
> wouldn't be used.
> Can you provide a use case where they could be used?


>
>
>> 7) The return type for RemoveLabel, RemoveLabelObject in
>> LabelMap class are void. STL map tells how many
>> objects are removed with the corresponding function prototype.
>> This information can be provided to the caller.
>>
>
> That would be a nice feature.
> Could you provide a patch or file a bug report ?
>
> Regards,
>
> Gaëtan
>
> --
> Gaëtan Lehmann
> Biologie du Développement et de la Reproduction
> INRA de Jouy-en-Josas (France)
> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
> http://mima2.jouy.inra.fr  http://www.itk.org
> http://www.bepo.fr
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-users/attachments/20120229/14f6a31e/attachment.htm>