kdesimone/popeye — pull request #28 by mekman (2014-10-27 10:23:04+00:00)
Title: Renaming class attributes of GaussianFit

Hi,

this implements the changes in the naming conventions for the ``GaussianFit`` class as discussed in #27. If you agree with the changes we could maybe change the ``DifferenceOfGaussianFit`` class in a separate PR.

Getting more familiar with the code base, I was wondering what the purpose of the ``auto-fit`` parameter is? I can't see a use-case for this to be ``False``. I'm probably missing something? In case this is only there for historical reasons would you agree to remove it - and consequently always fit the model when the class is called?

What do you think about renaming the parameter ``uncorrected_rval`` to something like ``rsquared_threshold``? I had a look at the Dumoulin paper again and they also express the threshold in terms of explained variance (r2). I personally find it more intuitive to think in terms of explained variance, and it would also be more consistent with the terminology in the paper. WDYT? Alternatively ``explained_variance_threshold`` would also work, but is IMHO a bit too long.

I think it would also make sense to have this parameter default to ``rsquared_threshold=None`` instead of ``0.0``. This would ensure that the second stage model fit is always performed. ATM, the first stage r-value can result in nan, in which case the second stage model fit is not performed.

(*NB*, the git log looks a bit bigger than it is, because I removed all trailing spaces in ``og.py`` (PEP8).)