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

Deprecate ResourceIdentifier methods which mutate resource string #2303

Open
wants to merge 4 commits into
base: master
from

Conversation

4 participants
@d-chambers
Copy link
Member

d-chambers commented Feb 7, 2019

What does this PR do?

  1. Depreciates methods of ResourceIdentifier which mutate the resource string in place.
  2. Adds a method get_quakeml_id for returning a new ResourceIdentifier instance with a valid quakeml uri.
  3. Rename get_quakeml_uri to get_quakeml_uri_str and depreciated the old methods usage
  4. Updated docs not to use the depreciated methods.

Why was it initiated? Any relevant Issues?

The ResourceIdentifier is hashable and as such any mutation of the resource string should be discouraged. Doing so already raises a warning so any internal methods that mutate the string should also be depreciated.

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 d-chambers added this to the 1.2.0 milestone Feb 7, 2019

@d-chambers

This comment has been minimized.

Copy link
Member Author

d-chambers commented Feb 11, 2019

+DOCS

@d-chambers d-chambers force-pushed the deprecate_id_mutation branch from 7cea2fb to c5723fe Feb 11, 2019

@krischer krischer added this to Waiting for Review in Release 1.2.0 Feb 14, 2019

@megies megies force-pushed the deprecate_id_mutation branch from c5723fe to 495d070 Feb 14, 2019

@megies

This comment has been minimized.

Copy link
Member

megies commented Feb 14, 2019

Rebased on current master and force-pushed so that we have fresh CI results tomorrow.

d-chambers added some commits Feb 7, 2019

@megies megies force-pushed the deprecate_id_mutation branch from 495d070 to 0f61245 Feb 14, 2019

@ThomasLecocq

This comment has been minimized.

Copy link
Contributor

ThomasLecocq commented Feb 15, 2019

the appveyor issue is the classic unrelated evalresp one.

@krischer this one is for you now.

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