Skip to content
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

TST: Update travis.yml #14915

Merged
merged 1 commit into from
Nov 18, 2019
Merged

TST: Update travis.yml #14915

merged 1 commit into from
Nov 18, 2019

Conversation

charris
Copy link
Member

@charris charris commented Nov 15, 2019

  • Use 3.8 instead of 3.8-dev
  • Use Python 3.7 for most tests.
  • Re-enable --durations except for debug runs
  • Add some blank lines for clarity

Perhaps we should spread the other tests across more Python versions?

@mattip
Copy link
Member

mattip commented Nov 15, 2019

i think it is good to thoroughly test one version. Is there a way to collapse all the python3.7 tests into one environment with many jobs to make it clear this is on purpose?

@charris
Copy link
Member Author

charris commented Nov 15, 2019

The different tests are run with different environment variables so NumPy still needs to be rebuilt. I think building and testing takes up most of the time. The pickle test should be covered by default in 3.8, so that probably doesn't need to be there. There may be some other tests that can be eliminated or combined but I don't want to experiment with those things in this PR. This is just to get the tests ready for the 1.8 branch.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

I've posted on the list for a discussion of tests we might want to eliminate.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

A bunch of resize tests are failing on Python3.7. Any idea why?

E           ValueError: cannot resize an array that references or is referenced
E           by another array in this way.
E           Use the np.resize function or refcheck=False

Seems to have come in in #12121.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

My guess is that the refcheck in the code is failing in 3.7. Trying now with 3.8 to see if that is still the case.

@mattip
Copy link
Member

mattip commented Nov 15, 2019

I think there is some strange interaction between pytest --coverage, the resize tests, and python > 3.6. Still trying to hunt it down.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

Just to summarize, the resize tests fail on 3.7 and 3.8, but pass on 3.6. The pytest, wheel, setup, etc. versions are all the same. The official wheel builds pass on all versions.

@mattip Yeah, it is probably one of the "extras" in that test run.

@mattip
Copy link
Member

mattip commented Nov 15, 2019

It is --coverage which changes the allocators and messes with refcounts.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

@mattip Thanks for tracking that down. How did you determine that?

@mattip
Copy link
Member

mattip commented Nov 15, 2019

git clean -xfd; CFLAGS='-O0 -g3' gdb --args python -b runtests.py --coverage -t numpy/core/tests/test_regression.py, then setting a breakpoint in shape.c:113 where we emit the error. The strange thing is, python sys.getrefcount(x) is reporting 2, but inside the debugger PyArray_REFCOUNT(self) is 3, so maybe coverage is also messing with sys.getrefcount?

@mattip
Copy link
Member

mattip commented Nov 15, 2019

It must be some kind of meta-programming, I don't see where coverage changes any of what I claim it does, but the refcount is definitely 3 in the gdb debugger

@mattip
Copy link
Member

mattip commented Nov 15, 2019

Here is an issue CPython had with coverage and refcounts, but they closed it in 2011

@charris
Copy link
Member Author

charris commented Nov 15, 2019

Given the Python dependency, I'm going to guess that something in sys.getrefcount changed. Or maybe something else used by coverage.

@charris
Copy link
Member Author

charris commented Nov 15, 2019

The short term fix looks to be using 3.6 for that test, that gives us some time to deal with the problem.

@@ -71,24 +78,30 @@ matrix:
- NPY_BLAS_ORDER=mkl,blis,openblas,atlas,accelerate,blas
- NPY_LAPACK_ORDER=MKL,OPENBLAS,ATLAS,ACCELERATE,LAPACK
Copy link
Member Author

Choose a reason for hiding this comment

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

Anyone know why these environmental variables were added? I don't think we use any of those.

@charris
Copy link
Member Author

charris commented Nov 17, 2019

Rebased.

- Use 3.8 instead of 3.8-dev
- Add some blank lines for clarity
@charris charris merged commit 5320157 into numpy:master Nov 18, 2019
@charris charris deleted the update-travis-3.8 branch November 18, 2019 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants