New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG+1] Drop NumPy < 1.8 #8874

Merged
merged 17 commits into from Jun 7, 2017

Conversation

Projects
None yet
4 participants
@naoyak
Contributor

naoyak commented May 13, 2017

ref: #8810
mostly removing old backports from utils.fixes

@naoyak naoyak changed the title from [WIP] Drop NumPy < 1.8 to Drop NumPy < 1.8 May 14, 2017

@naoyak naoyak changed the title from Drop NumPy < 1.8 to MAINT Drop NumPy < 1.8 May 14, 2017

@naoyak naoyak referenced this pull request May 14, 2017

Merged

[MRG+1] MAINT drop SciPy < 0.13 #8854

8 of 8 tasks complete
@lesteve

This comment has been minimized.

Member

lesteve commented Jun 1, 2017

Skimming this PR this looks good overall. Could you keep the numpy-related stuff only in this PR? I feel the review process will be easier this way.

@lesteve

This comment has been minimized.

Member

lesteve commented Jun 1, 2017

I'll start looking at #8854 first, which does the scipy-related removal/deprecation, and then tackle this one.

@naoyak

This comment has been minimized.

Contributor

naoyak commented Jun 1, 2017

Yeah, I figured it might be easier to get #8854 in and then rebase - doing these in parallel would create some merge conflicts.

@naoyak naoyak closed this Jun 2, 2017

@naoyak naoyak reopened this Jun 2, 2017

@naoyak naoyak changed the title from MAINT Drop NumPy < 1.8 to [MRG] Drop NumPy < 1.8 Jun 2, 2017

@naoyak

This comment has been minimized.

Contributor

naoyak commented Jun 2, 2017

Rebased and ready for review.

@naoyak naoyak closed this Jun 3, 2017

@naoyak naoyak reopened this Jun 3, 2017

@naoyak naoyak closed this Jun 4, 2017

@naoyak naoyak reopened this Jun 4, 2017

@jnothman

The changes LGTM though I've not checked for completeness

@jnothman jnothman changed the title from [MRG] Drop NumPy < 1.8 to [MRG+1] Drop NumPy < 1.8 Jun 4, 2017

@naoyak

This comment has been minimized.

Contributor

naoyak commented Jun 6, 2017

I added a commit updating Travis config for Anaconda 4.4.0, can also move that to a separate PR.

@amueller

This comment has been minimized.

Member

amueller commented Jun 6, 2017

travis fails. Oh, actually codecov, never mind.

.travis.yml Outdated
COVERAGE=true
# This environment tests the oldest supported anaconda env
- DISTRIB="conda" PYTHON_VERSION="2.7" INSTALL_MKL="false"
NUMPY_VERSION="1.8.2" SCIPY_VERSION="0.13.3" CYTHON_VERSION="0.23.4"
NUMPY_VERSION="1.8.2" SCIPY_VERSION="0.13.3" CYTHON_VERSION="0.23.5"

This comment has been minimized.

@amueller

amueller Jun 6, 2017

Member

Wait, we didn't test <1.8 earlier?

This comment has been minimized.

@lesteve

lesteve Jun 6, 2017

Member

@we did with the Ubuntu Precise build. Edit: and also one of the conda build.

This comment has been minimized.

@lesteve

lesteve Jun 6, 2017

Member

Note the PR updating the dependencies on Travis by @jnothman: #8855

@@ -431,7 +429,7 @@ def fit(self, X, y):
if self.priors is None: # estimate priors from sample
_, y_t = np.unique(y, return_inverse=True) # non-negative ints
self.priors_ = bincount(y_t) / float(len(y))
self.priors_ = np.bincount(y_t) / float(len(y))

This comment has been minimized.

@amueller

amueller Jun 6, 2017

Member

this never even needed the backport? huh

@amueller

This comment has been minimized.

Member

amueller commented Jun 6, 2017

can you add an entry to whatsnew with all the function that were removed?

@amueller

This comment has been minimized.

Member

amueller commented Jun 6, 2017

LGTM if whatsnew entry is added

@lesteve

This comment has been minimized.

Member

lesteve commented Jun 6, 2017

LGTM if whatsnew entry is added

Agreed. You should do a single entry for the both numpy + scipy dependency upgrade. I would mention:

  • new numpy and scipy minimal required versions
  • list all the functions that have been removed from sklearn.utils.fixes
  • maybe list all the functions that have been deprecated, although the deprecated warnings may be enough, not sure
@amueller

This comment has been minimized.

Member

amueller commented Jun 6, 2017

+1 for adding a list of deprecated functions. That makes reconstructing history easier if someone is using an old version etc.

@lesteve

This comment has been minimized.

Member

lesteve commented Jun 6, 2017

Hmmm looks like you need to rebase on master and fix the conflicts.

@naoyak

This comment has been minimized.

Contributor

naoyak commented Jun 6, 2017

@amueller thanks, rebased with whatsnew entry added

@jnothman

This comment has been minimized.

Member

jnothman commented Jun 7, 2017

What's new LGTM

.travis.yml Outdated
@@ -46,7 +46,7 @@ matrix:
PANDAS_VERSION="0.20.1" CYTHON_VERSION="0.25.2"
# flake8 linting on diff wrt common ancestor with upstream/master
- env: RUN_FLAKE8="true" SKIP_TESTS="true"
DISTRIB="conda" PYTHON_VERSION="3.6.1" INSTALL_MKL="true"

This comment has been minimized.

@amueller

amueller Jun 7, 2017

Member

that's odd but whatever.

@amueller

This comment has been minimized.

Member

amueller commented Jun 7, 2017

should we have upgraded the flake8 version instead of downgraded the python version?

@naoyak

This comment has been minimized.

Contributor

naoyak commented Jun 7, 2017

should we have upgraded the flake8 version instead of downgraded the python version?

I don't think this is resolved in flake8 yet: 80c1bf1

@lesteve

This comment has been minimized.

Member

lesteve commented Jun 7, 2017

I am going to merge this one without waiting for AppVeyor.

Thanks a lot @naoyak !

@lesteve lesteve merged commit 6579220 into scikit-learn:master Jun 7, 2017

2 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@naoyak naoyak deleted the naoyak:numpy-deps branch Jun 10, 2017

Sundrique added a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017

dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017

dmohns added a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017

NelleV added a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017

paulha added a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017

AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment