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.

Sign up for GitHub

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

Jump to bottom

Made Py3.6 main language #1517

Merged
merged 5 commits into from
Jul 22, 2017
Merged

Made Py3.6 main language #1517

merged 5 commits into from
Jul 22, 2017

Conversation

richardjgowers
Copy link
Member

@richardjgowers richardjgowers commented Jul 18, 2017 •
edited
Loading

Fixes #1525

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@richardjgowers richardjgowers force-pushed the py3builds branch 2 times, most recently from fd8e444 to 45d93d1 Compare July 18, 2017 19:21
@kain88-de
Copy link
Member

I don't like that we have more then one osx test. One osx test would be enough.

@richardjgowers
Copy link
Member Author

@kain88-de we should really do them all, but maybe pragmatically we can just do the 3.6 one

@richardjgowers
Copy link
Member Author

Or maybe making the osx builds a cron job, so we get a delayed response to them failing

@kain88-de
Copy link
Member

What should fail with them?

@tylerjereddy
Copy link
Member

Lots of passes (good), a few strange hiccups, and maybe some minor issues with the new Py version linting in this PR based on initial discussion with @richardjgowers

@richardjgowers richardjgowers changed the title Added python3 to build matrix Made Py3.6 main language Jul 19, 2017
@tylerjereddy
Copy link
Member

I think we're seeing the pytest / xdist multicore error Different tests were collected between gw1 and gw0 consistently here for some rows because @richardjgowers already tried restarting those specific failures. See my related note in a more recent build matrix PR.

kain88-de
kain88-de reviewed Jul 20, 2017
View reviewed changes
.travis.yml Outdated
include:
- os : linux
env: NAME='minimal'
MEMLEAK='--with-memleak'
PYTHON_VERSION=3.6
Copy link
Member

Choose a reason for hiding this comment

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

it shouldn't be necessary that you specify the default version all the time.

@kain88-de kain88-de force-pushed the py3builds branch 3 times, most recently from 09be9da to d8692a1 Compare July 20, 2017 17:02
@richardjgowers richardjgowers added this to the 0.17.0 milestone Jul 20, 2017
@kain88-de
Copy link
Member

We still depend on nose for a lot of tests that try to load it at runtime when we call specific functions.

kain88-de
kain88-de reviewed Jul 20, 2017
View reviewed changes
.travis.yml Outdated

- env: NAME='minimal'
CONDA_DEPENDENCIES=${CONDA_MIN_DEPENDENCIES}
MAIN_CMD="pytest testsuite/MDAnalysisTests/analysis"
Copy link
Member

Choose a reason for hiding this comment

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

Also minimal now only runs on the analysis module. This should keep the time at a minimum

@kain88-de
Copy link
Member

Here are the nose errors that we still have: https://travis-ci.org/MDAnalysis/mdanalysis/jobs/255755498

@kain88-de
Copy link
Member

Feel free to squash this to one commit during a merge. I've committed a lot of small test changes here.

@richardjgowers
Copy link
Member Author

@kain88-de do you mean that we don't test core on minimal?

@kain88-de
Copy link
Member

Yes.

@kain88-de
Copy link
Member

I can revert that again.

@richardjgowers
Copy link
Member Author

@kain88-de yeah I know we're not meant to use weird imports outside of analysis, but I'd rather keep the check there just in case we miss something

@kain88-de
Copy link
Member

kain88-de commented Jul 20, 2017 •
edited
Loading

No you are right. If anything it should be test all but the analysis module

richardjgowers and others added 2 commits July 20, 2017 21:40
@richardjgowers @kain88-de
Added python3 to build matrix
179444d
@kain88-de
general clean up of travis.yml
0e2745f 
- fix hole installation for max
- simplify test scripts
@kain88-de kain88-de mentioned this pull request Jul 20, 2017
Closed
@kain88-de
Copy link
Member

@orbeckst there is a problem installing hole on osx

@orbeckst
Copy link
Member

here is a problem installing hole on osx

I have to do some other boring urgent things this afternoon so not much time right now, can you be more specific if you have a moment?

@kain88-de
Copy link
Member 

Downloading hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz from https://www.dropbox.com/s/5mzrsyp48i32je4/hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz?dl=1...

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current

                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0

curl: (77) error setting certificate verify locations:

  CAfile: /etc/ssl/certs/ca-certificates.crt

  CApath: none

[install_hole.sh] ERROR: Failed to download hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz from https://www.dropbox.com/s/5mzrsyp48i32je4/hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz?dl=1 [1]

looks like we don't have the correct binary for travis osx

@kain88-de
skip hole on osx for now.
b71471b 
we don't have the correct binary to download
@orbeckst
Copy link
Member

orbeckst commented Jul 21, 2017 •
edited
Loading

I think the hole download issue is a problem with TLS certificates and curl. We can try

curl --insecure  ...

or perhaps the workaround with

# fixing curl on travis/anaconda, see http://stackoverflow.com/a/31060428/334357
export CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt

is not needed anymore (possibly, the system curl in Mac OSX knows to get the certs from the Apple Keychain) and everything "just works" when we remove this line.

EDIT:

One suggestion is to rely on the system curl and

conda remove curl

@kain88-de
Copy link
Member

We might need to rethink the python 3.4 CI tests. Conda-forge and anaconda only support the last two stable release branches 3.6 and 3.5. This results for example in numpy 1.13 not being build for python 3.4 as a conda package, and conflicts installing our dependencies. In turn conflicts during installation are not good because this means we have to wait ages for conda to finish and might cross the 50min mark to single jobs on travis.

kain88-de
kain88-de reviewed Jul 21, 2017
View reviewed changes
.travis.yml Outdated
@@ -28,7 +28,7 @@ env:
- BUILD_CMD="pip install -v package/ && pip install testsuite/"
# we need nose for assert_raises and stuff
- CONDA_MIN_DEPENDENCIES="mmtf-python mock six biopython networkx cython joblib matplotlib scipy griddataformats hypothesis nose"
- CONDA_DEPENDENCIES="${CONDA_MIN_DEPENDENCIES} seaborn clustalw=2.1 netcdf4 scikit-learn"
Copy link
Member

Choose a reason for hiding this comment

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

Hups coveralls is not equal to coverage. Sorry

@richardjgowers
Copy link
Member Author

this is passing, can someone merge this?

@kain88-de kain88-de merged commit e0bc303 into develop Jul 22, 2017
@kain88-de kain88-de deleted the py3builds branch July 22, 2017 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kain88-de kain88-de kain88-de approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@richardjgowers @kain88-de @tylerjereddy @orbeckst