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

Stream.rotate('->ZNE', components=...): be nice on slightly unexpected input #2082

Merged
merged 2 commits into from Mar 2, 2018

Conversation

Projects
None yet
2 participants
@megies
Member

megies commented Mar 1, 2018

see
http://lists.swapbytes.de/archives/obspy-users/2018-March/002692.html

What does this PR do?

Make Stream.rotate('->ZNE', ...) accept components that is a string of component codes (as opposed to a list of strings). Although it's clear from the docstring what is expected, I think the likelihood of people misusing this parameter are high enough that we should work around it. A warning is also shown on invalid input before it is converted.

Why was it initiated? Any relevant Issues?

Report on users mailing list, see http://lists.swapbytes.de/archives/obspy-users/2018-March/002692.html

PR Checklist

  • Correct base branch selected? master for new fetures, 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"
    no docs change
  • 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)
    no network tests
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests. no new features
  • Any new or changed features have are fully documented. no new features
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .

@megies megies added the .core label Mar 1, 2018

@megies megies added this to the 1.1.1 milestone Mar 1, 2018

if isinstance(components, (str, native_str)):
msg = ("'components' is expected to be a list of one or more "
"component code combinations (e.g. `components=['Z12']` or "
"`components=['Z12', '123']`)")

This comment has been minimized.

@krischer

krischer Mar 1, 2018

Member

The warning seems superfluous to me. Once we add this capability we'll not remove it again without good reason and the code works perfectly fine so its IMHO valid code which should raise no warning. I would just change the docstring to state that single strings are also allowed.

This comment has been minimized.

@megies

megies Mar 2, 2018

Member

fair enough, I guess

changes applied

@megies megies merged commit a0fb796 into maintenance_1.1.x Mar 2, 2018

5 of 6 checks passed

docker-testbot docker testbot results not available yet
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 90%)
Details
codecov/project 88.08% (+1.16%) compared to 2537cd5
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@megies megies deleted the rotate_zne branch Mar 2, 2018

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