Skip to content

MNT: drop 3.5 testing for 3.1 branch #12538

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

Merged
merged 5 commits into from
Dec 5, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Oct 16, 2018

PR Summary

As per dev-list email, dropping 3.5 tests from Appveyor and Travis...

Feel free to push or close this if I did it incorrectly!

Probably todo is Circle-ci. Do we want to add 3.7 to that?

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jklymak jklymak added this to the v3.1 milestone Oct 16, 2018
@jklymak jklymak closed this Oct 16, 2018
@jklymak jklymak reopened this Oct 16, 2018
@tacaswell
Copy link
Member

The 3.5 test was pinning all of the minimum deps, we need to move those over to one of the other tests.

dist: trusty
# pytest-cov>=2.3.1 due to https://github.com/pytest-dev/pytest-cov/issues/124.
env:
- PINNEDVERS='-c requirements/testing/travis35.txt'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also remove this file (or rename it to be the 3.6 pins).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were pinning numpy to 1.10 on the other builds?

Copy link
Member Author

@jklymak jklymak Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the Travis 3.6 build was getting reqs from travis_all.txt, so I think this is OK... Oh duh, I see we need to make sure one of the tests tests the minimums. OK. Thats a bit more work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, see new file. I'm not sure if the other pinned reqs in travis35.txt were required to get it to run on 3.5 or if they were min deps we were wanting to test. I suspect the former, so I removed them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest and sphinx are pinned as minimum deps also.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that? Are we testing that devs can use old Sphinx and pytest? Or are those user facing pins?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so; those are the versions we specify in the docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, - wasn't sure if w need the pin on pytest-timeout so trying w/o

@jklymak
Copy link
Member Author

jklymak commented Oct 17, 2018

Looks done to me, but let me know if I missed something!

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to be the one to merge this so abuse the the 'request changes' feature to that end.

tacaswell
tacaswell previously approved these changes Dec 3, 2018
timhoffm
timhoffm previously approved these changes Dec 3, 2018
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

pandas<0.21.0
pytest==3.6
pytest-cov==2.3.1
# pytest-timeout==1.2.1 # Newer pytest-timeouts don't support pytest 3.4.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed instead of uncommented.

@jklymak jklymak force-pushed the mnt-drop35-travis branch 2 times, most recently from 28352de to 2ad5f6e Compare December 3, 2018 21:03
@jklymak
Copy link
Member Author

jklymak commented Dec 3, 2018

Sorry @timhoffm and @tacaswell - I messed up the rebase and had to do again. Clearing your approvals pending a new quick re-review

@jklymak jklymak dismissed stale reviews from timhoffm and tacaswell December 3, 2018 21:05

stale after bad rebase.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

travis36minver.txt is missing

@jklymak
Copy link
Member Author

jklymak commented Dec 3, 2018

Should there be a ci/circleci:docs-python37? I'm not really clear on the point of running two doc builds.

@jklymak
Copy link
Member Author

jklymak commented Dec 3, 2018

Fixed now - sorry for the confusion!

@@ -121,39 +121,6 @@ jobs:
name: "Deploy new docs"
command: ./.circleci/deploy-docs.sh

docs-python35:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move to a 3.7 docker build, and pin numpy 1.11 on the 3.6 build?
The CircleCI build does complement the coverage of the test suite.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, but does doing it on 3.6 and 3.7 buy us that much?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test with oldest numpy (1.11) on 3.6, latest on 3.7.

@@ -0,0 +1,6 @@
# Extra pip requirements for the first travis python 3.6 build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to delete travis35.txt no?

@tacaswell
Copy link
Member

pushed one more commit to (I think) get the circle 37 job to actually run!

@jklymak
Copy link
Member Author

jklymak commented Dec 4, 2018

Thanks @tacaswell. Not sure I understand the codecov complaint...

@jklymak
Copy link
Member Author

jklymak commented Dec 4, 2018

I wonder if the azure pip install requirements need to be tweaked? And should the azure 3.6 test pin numpy to 1.11?

@tacaswell
Copy link
Member

As mentioned on the call we need to look at moving all the tests over to azure so I'm less worried about getting it's pinnings right yet.

@tacaswell
Copy link
Member

In test_animation we no longer hit the 'false' path on that conditional. Not sure on the other one yet but I suspect it is something similar.

@jklymak
Copy link
Member Author

jklymak commented Dec 4, 2018

Fixed test_animation. No idea how to fix backend_ps...

@jklymak
Copy link
Member Author

jklymak commented Dec 4, 2018

Fixed itself 😉

@tacaswell tacaswell merged commit e2045c3 into matplotlib:master Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants