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

Add confidence ellipsoid reading #3350

Merged
merged 7 commits into from May 17, 2024
Merged

Add confidence ellipsoid reading #3350

merged 7 commits into from May 17, 2024

Conversation

calum-chamberlain
Copy link
Contributor

@calum-chamberlain calum-chamberlain commented Sep 20, 2023

What does this PR do?

This PR adds reading confidence ellipsoids directly from NLLoc hyp files to ObsPy events. This also copes with the confidence ellipsoid not being included in the NLLoc hyp files.

On opening I haven't included a test for this yet, but will add one soon.

Why was it initiated? Any relevant Issues?

NLLoc hyp files (at least for NLLoc v7) provide this and it was otherwise silently skipped

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).
  • While the PR is still work-in-progress, the no_ci label can be added to skip CI builds
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just add the build_docs tag to this PR.
    Docs will be served at docs.obspy.org/pr/{branch_name} (do not use master branch).
    Please post a link to the relevant piece of documentation.
  • If all tests including network modules (e.g. clients.fdsn) should be tested for the PR,
    just add the test_network tag to this PR.
  • All tests still pass.
  • Any new features or fixed regressions are covered via new tests.
  • Any new or changed features are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
  • If the changes affect any plotting functions you have checked that the plots
    from all the CI builds look correct. Add the "upload_plots" tag so that plotting
    outputs are attached as artifacts.
  • New modules, add the module to CODEOWNERS with your github handle
  • Add the yellow ready for review label when you are ready for the PR to be reviewed.

@calum-chamberlain calum-chamberlain added ready for review PRs that are ready to be reviewed to get marked ready to merge and removed no_ci labels Sep 20, 2023
@megies megies added this to the 1.5.0 milestone Sep 21, 2023
CHANGELOG.txt Outdated Show resolved Hide resolved
obspy/io/nlloc/core.py Outdated Show resolved Hide resolved
obspy/io/nlloc/core.py Outdated Show resolved Hide resolved
Copy link
Member

@megies megies left a comment

Choose a reason for hiding this comment

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

A couple of minor comments, tests need fixing (see comment), otherwise looking good to me. 🚀

@megies megies force-pushed the nlloc_confidence_ellipsoid branch from 6258fc8 to e70b1a1 Compare May 17, 2024 06:54
@megies
Copy link
Member

megies commented May 17, 2024

Rebased and final touches

@megies megies merged commit 02b38a9 into master May 17, 2024
29 checks passed
@megies megies deleted the nlloc_confidence_ellipsoid branch May 17, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.io.nlloc ready for review PRs that are ready to be reviewed to get marked ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants