> I am by no means committed to keeping the current implementation, I agree that it would be nice to get rid of these wrappers. I think it would be better to wait till post merge given the above two considerations (and perhaps after the initial release) but I will do whichever the group decides. 

:+1:, this is tough in the sense that not keeping these wrappers would
require to replicate these few lines all over the code-base making it
rather unreadable unless the same comments were repeated all over the
place.  The other alternative is to have these wrappers live inside
qiime/stats.py now that qiime depends on scipy, it should be transparent
right?

On (May-14-14|16:52), Will Van Treuren wrote:
> > -        r = 1.0 * ((n * sum_xy) - (sum_x * sum_y)) / \
> > -            (np.sqrt((n * sum_x_sq) - (sum_x * sum_x))
> > -             * np.sqrt((n * sum_y_sq) - (sum_y * sum_y)))
> > -    except (ZeroDivisionError, ValueError, FloatingPointError):
> > -        # no variation
> > -        r = 0.0
> > -    # check we didn't get a naughty value for r due to rounding error
> > -    if r > 1.0:
> > -        r = 1.0
> > -    elif r < -1.0:
> > -        r = -1.0
> > -    return r
> > -
> > -
> > -def spearman(x_items, y_items):
> > +def pearson(v1, v2):
> 
> I agree that these are ultimately something that should be removed. The reason I did not is twofold:
> 
> 1. It would necessitate a large number of changes in the interface for ```group_significance.py``` and I believe other scripts that use Pearson, Spearman, or Kendall. 
> 
> 2. The p-value calculations that are made by these scripts in scipy (```scipy.stats.pearsonr, scipy.stats.spearmanr, scipy.stats.kendalltau```) are not necessarily the p-value calculations we want. I have not checked the ```scipy``` implementations vs. the implementations that we are currently using from Biometry, but I don't think they are the same. For instance, the ```scipy.stats.pearsonr``` documentation indicates that 
> ```The p-value roughly indicates the probability of an uncorrelated system
> producing datasets that have a Pearson correlation at least as extreme
> as the one computed from these datasets. The p-values are not entirely
> reliable but are probably reasonable for datasets larger than 500 or so.```
> 
> Biometry suggests we can do far better than this with the Fisher Z transform. 
> 
> I am by no means committed to keeping the current implementation, I agree that it would be nice to get rid of these wrappers. I think it would be better to wait till post merge given the above two considerations (and perhaps after the initial release) but I will do whichever the group decides. 
> 
> 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/biocore/scikit-bio/pull/342/files#r12668438