[Biopython-dev] [Bug 2697] MaxEntropy calculate function assumes integer values for class and convergence criteria is hard coded

[bugzilla-daemon at portal.open-bio.org]

http://bugzilla.open-bio.org/show_bug.cgi?id=2697





------- Comment #11 from biopython-bugzilla at maubp.freeserve.co.uk  2009-02-16 18:14 EST -------
(In reply to comment #10)
> 
> My patch removed these hard coded default values and placed them in the
> function. 
> 

Yes - and in doing so it broke the existing API.  My way preserves the ability
to alter these module level variables as another way to control the funtion. 
We could describe these old upper case module level variables as obsolete in
the docstring as a step to phasing them out.

> >The patch deals with this as follows:
> > 
> > def train(training_set, results, feature_fns, update_fn=None,
> >           max_iis_iterations=None, iis_converge=None,
> >           max_newton_iterations=None, newton_converge=None):
> >     if max_iis_iterations is None :
> >         max_iis_iterations = MAX_IIS_ITERATIONS
> >     if iis_converge is None :
> >         iis_converge = IIS_CONVERGE
> >     if max_newton_iterations is None :
> >         max_newton_iterations = MAX_NEWTON_ITERATIONS
> >     if newton_converge is None :
> >         newton_converge = NEWTON_CONVERGE
> > 
> > This works :)
> 
> 
> I hate the use of the local variable being the lowercase version of another
> variable. Obviously for the original variables we are stuck with uppercase for
> backwards compatibility. So we need to change the names of the lowercase
> variables.

Hate is a rather strong word.  I can see that having the same name except for
the case could confuse some people if they are not used to case mattering, but
otherwise using the same name seems like a GOOD idea to me for consistency.

Do you have any concrete suggestions?  We could expand ISS into words.  On a
related point, using a lower case N for Newton feels a bit wrong to me ;)


-- 
Configure bugmail: http://bugzilla.open-bio.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.