Closed scottclowe closed 2 years ago
Merging #271 (de882ba) into master (6b3b320) will increase coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #271 +/- ##
==========================================
+ Coverage 94.29% 94.30% +0.01%
==========================================
Files 8 8
Lines 1209 1212 +3
Branches 310 311 +1
==========================================
+ Hits 1140 1143 +3
Misses 35 35
Partials 34 34
Flag | Coverage Δ | |
---|---|---|
nbsmoke | 60.31% <100.00%> (+0.09%) |
:arrow_up: |
unittests | 93.97% <100.00%> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
fissa/neuropil.py | 95.38% <100.00%> (+0.22%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 6b3b320...de882ba. Read the comment docs.
In version 1.0.0, sklearn deprecated the
alpha
argument to NMF in favour of thealpha_W
andalpha_H
arguments, which were added in v1.0.0. The deprecation warning declares thatalpha
will be removed in v1.2.0.We can't simply swap to using
alpha_W
instead ofalpha
because the behaviour is different:I imagine the new method where the alphas scale against the number of features/samples is better... but we selected
alpha=0.1
without any such scaling and it is not clear to me without running some tests that we should usealpha_W=alpha_H=0.1
.Another issue is that v1.0.0 which added
alpha_W
andalpha_H
only support python >= 3.7, so we would have to cut support for python <= 3.6 to move to the new versions of alpha.Until someone investigates a new appropriate value of alpha, for the meantime we will carry on using the old version of alpha, suppress the deprecation warning so that end-users do not have to tolerate it, and declare that FISSA requires sklearn<1.2.