[Insight-developers] PathToImageFilter issues

[Miller, James V (GE, Research)]

Zachary, 

I wasn't really suggesting the new code (with all your refactoring)
would be something that the IJ would build.  I was just suggesting
it as a place to the code could be posted for others to review. 
I imagine if there is no CMakeLists.txt file, then the IJ
will not attempt to build the code. The benefit of the IJ is that 
you could also post a document describing the new code and the
refactorings.

I'd prefer the code posted as complete files rather than patches. 
My ability to browse or apply a patch on my Windows system has
never been resurrected from when I replaced my laptop last year.

An alternative is to post these to the bug tracker.  I just thought
the IJ with its document management system would be a better place.

Another alternative is create a (short lived) branch on which you
put the code.

Jim


-----Original Message-----
From: Zachary Pincus [mailto:zpincus at stanford.edu]
Sent: Monday, February 13, 2006 1:55 PM
To: Miller, James V (GE, Research)
Cc: insight-developers at itk.org; Lorensen, William E (GE, Research)
Subject: Re: [Insight-developers] PathToImageFilter issues


Hi folks -

It might be good to get these fixes into the next revision of ITK.  
Given that the fix necessarily involves deprecating some  
functionality and moving it to another class, it might be good to get  
people using the proper class sooner rather than later. More  
importantly, the current PathToImageFilter "base class" is impossible  
to properly subclass from without these fixes. However, you guys  
probably have enough to do already, so I'll defer to you on this.

In general, I'm not totally sure the IJ is the best forum for  
reviewing maintenance fixes to existing ITK code, especially with  
regard to hewing to ITK conventions. (Note that the  
PathToImageFilter, cause of these issues, is part of the second- 
highest-ranked submission to the IJ.)

For one, the IJ build environment doesn't have (as far as I can tell)  
a CVS version of ITK, so you can't submit code and be sure that it  
will work with the latest ITK. Moreover, submitted code can't use  
things only in the CVS. Both of these are obviously a must for bug- 
fix issues. (Already with my previous IJ submission, there are some  
things that will have to be changed when/if it is incorporated into  
ITK; e.g. the CVS has the more expressive  
"InvalidRequestedRegionError" and 2.4.1 does not.)

Moreover, there's no mechanism in IJ to submit patches to code  
already in the CVS. Thus, to discuss these changes, I'd have to  
submit new Path and PathToImageFilter classes to the IJ, and hope the  
include paths on the build machine get set correctly so the new  
classes are found instead of the current versions of the same. Then  
to integrate the changes into ITK, someone would have to manually  
diff them against the current version and roll the changes in, and so  
forth.

Perhaps there could be a lighter-weight mechanism for reviewing bug- 
fix changes, etc? I agree that peer-review of code (bug-fixes and new  
submissions) is important, but while using the IJ to "raise the bar"  
for new submissions (in terms of quality and usefulness) is clearly a  
good idea, I'm not sure that raising the bar for people trying to fix  
existing problems is really the best approach. Likewise, I'm not sure  
that my constant spamming of the dev list proposing bug fixes is  
optimal either!

Zach



On Feb 13, 2006, at 11:49 AM, Miller, James V (GE, Research) wrote:

> Zachary,
>
> I think posting the classes on the Insight Journal would allow a  
> number of people
> to evaluate the changes.  Already I would be tempted to rename  
> TracePathOnImageFilter
> to TracePathImageFilter.
>
> We are about to lock down the repository (if it wasn't already done  
> last night)
> in preparation of the next release.  Your changes may have to wait  
> until the
> repository re-opens after the release is tagged.
>
> Jim
>
> -----Original Message-----
> From: insight-developers-bounces+millerjv=crd.ge.com at itk.org
> [mailto:insight-developers-bounces+millerjv=crd.ge.com at itk.org]On  
> Behalf
> Of Zachary Pincus
> Sent: Monday, February 13, 2006 11:33 AM
> To: insight-developers at itk.org
> Cc: Lorensen, William E (GE, Research)
> Subject: Re: [Insight-developers] PathToImageFilter issues
>
>
> Hi folks,
>
> I've re-worked (in what I think is a 100% backward-compatible way)
> the PathToImageFilter class to deal with the issues that I brought up
> last week. Should I go ahead and commit the new class (and a new
> subclass, TracePathOnImageFilter, which has the functionality that
> has been deprecated from PathToImageFilter)?
>
> Perhaps it would be better if I send the classes to someone else for
> review to make sure I haven't broken compatibility, abused ITK
> convention, etc? (To avoid a re-play of the issue of having an
> incorrectly-defined base class in the future.) Any volunteers?
>
> Zach
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers