[MRG+1] Add logsumexp and comb to utils.fixes (SciPy 0.19+)#9046
[MRG+1] Add logsumexp and comb to utils.fixes (SciPy 0.19+)#9046agramfort merged 1 commit intoscikit-learn:masterfrom
Conversation
sklearn/decomposition/online_lda.py
Outdated
| try: # SciPy >= 0.19 | ||
| from scipy.special import logsumexp | ||
| except ImportError: | ||
| from scipy.misc import logsumexp |
There was a problem hiding this comment.
i would uses fixes.py for this to avoid the try except in all our examples
you would write:
from sklearn.utils import logsumexp
sklearn/utils/tests/test_random.py
Outdated
| try: # SciPy >= 0.19 | ||
| from scipy.special import comb | ||
| except ImportError: | ||
| from scipy.misc import comb |
There was a problem hiding this comment.
same remark here. Please put it in fixes.py
|
@agramfort thanks, moved to |
|
That means that we now have |
| from scipy.misc import comb | ||
| from scipy.sparse import csr_matrix | ||
| from scipy.sparse import csc_matrix | ||
| from scipy.sparse import coo_matrix |
There was a problem hiding this comment.
we typically don't import everything in one line to avoid too many changed lines when adding or removing some imports.
please revert
I did not really say that. I just said that it was a bit unfortunate that:
This leaves us with two I don't really like the alternative which is to put |
|
Looks like Travis errors are genuine. |
|
@lesteve's point is well taken - it's weird to have the same We could just move the try-except block to the deprecated |
| from scipy.sparse.linalg import lsqr as sparse_lsqr # noqa | ||
|
|
||
|
|
||
| try: # SciPy >= 0.19 |
There was a problem hiding this comment.
Should you not import from utils.fixes here?
There was a problem hiding this comment.
? this is in utils.fixes..
There was a problem hiding this comment.
Oops my bad, just ignore this ...
|
Out of interest do you close and reopen to re-trigger the CIs? You know you can push an empty commit or amend the commit and push -f to do this? The latter solution is one email instead of two ... |
|
Oh sorry, I use both methods (often close-reopen when I'm not at my computer...) but I'll amend as much as possible to minimize spam, thanks for the heads up. |
|
No worries, I also wanted to double-check with you that close + open was indeed restarting the CIs. |
|
LGTM |
|
thanks @naoyak |
Add import checks forAddlogsumexpandcombtoutils.fixes, since they have been moved (with deprecation) fromscipy.misctoscipy.specialas of SciPy 0.19.