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

Fix taup py37 failures #2280

Merged
merged 5 commits into from Jan 10, 2019

Conversation

Projects
None yet
3 participants
@d-chambers
Copy link
Member

d-chambers commented Jan 9, 2019

What does this PR do?

I am attempting to fix the python 3.7 TauP failure on Travis in the least invasive way possible.

Why was it initiated? Any relevant Issues?

#2269 fixed a lot of failures on master but we weren't sure how to fix this one.

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just remove the space in the following string after the + sign: "+ DOCS"
  • If any network modules should be tested for the PR, add them as a comma separated list
    (e.g. clients.fdsn,clients.arclink) after the colon in the following magic string: "+TESTS:"
    (you can also add "ALL" to just simply run all tests across all modules)
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
  • Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .

d-chambers added some commits Jan 9, 2019

@d-chambers

This comment has been minimized.

Copy link
Member

d-chambers commented Jan 9, 2019

@calum-chamberlain, what do you think of this? It is a bit of a hack but since no TauP experts are forthcoming this seems like a reasonable compromise. Hopefully all tests turn green after this change.

@d-chambers d-chambers changed the title wip: taup py37 failures taup py37 failures Jan 9, 2019

@d-chambers d-chambers changed the title taup py37 failures Fix taup py37 failures Jan 9, 2019

@calum-chamberlain

This comment has been minimized.

Copy link
Contributor

calum-chamberlain commented Jan 9, 2019

This looks good to me.

If we are uncertain about whether the implementation will impact anyones codes then we could warn that the reference to the original array is lost. It might also be worth linking to this PR so that if an issue does arise then the user can quickly find out why. I don't really like that idea, but its more of a "covering-our-backs" thing.

@d-chambers

This comment has been minimized.

Copy link
Member

d-chambers commented Jan 10, 2019

Yes, it is probably better to annoy than to surprise.

@calum-chamberlain

This comment has been minimized.

Copy link
Contributor

calum-chamberlain commented Jan 10, 2019

The appveyor fail is an unrelated fail as far as I know - trace similarity things as we also discussed in #2269
Looks good to me!

@megies

This comment has been minimized.

Copy link
Member

megies commented Jan 10, 2019

Thanks @d-chambers!

@megies megies merged commit 4c45419 into master Jan 10, 2019

2 of 4 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
docker-testbot docker testbot results not available yet
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@megies megies deleted the fix_taup_py37 branch Jan 10, 2019

@megies megies added this to the 1.2.0 milestone Jan 10, 2019

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