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

Remove error and add a warning for gravmag.transform.upcontinue when height <= 0 #337

Merged
merged 4 commits into from Nov 25, 2016

Conversation

Projects
None yet
2 participants
@rafaelmds
Member

rafaelmds commented Nov 24, 2016

Fixes #235

Change the behavior of gravmag.transform.upcontinue. Now, instead of an error its gives a warnings.warn when height <= 0.
This change is mainly for educational purposes (you can test what happen if you calculate for it for height <= 0).

Checklist:

  • Make tests for new code (at least 80% coverage)
  • Create/update docstrings
  • Include relevant equations and citations in docstrings
  • Docstrings follow the style conventions
  • Code follows PEP8 style conventions
  • Code and docs have been spellchecked
  • Include new dependencies in doc/install.rst, environment.yml, ci/requirements-conda.txt and ci/requirements-pip.txt.
  • Documentation builds properly (run cd doc; make locally)
  • Changelog entry (leave for last to avoid conflicts)
@leouieda

This comment has been minimized.

Member

leouieda commented Nov 25, 2016

@rafaelmds thanks for the PR! Could you please add the following code to fatiando/gravmag/tests/test_transform.py? The function should go below the test_upcontinue function and the import at the top. This will test if the warning is being raised properly.

import pytest


def test_upcontinue_warning():
    "gravmag.transform upward continuation raises warning if height <= 0"
    model = [Prism(-1000, 1000,-500, 500, 0, 1000, {'density': 1000})]
    shape = (100, 100)
    x, y, z = gridder.regular([-5000, 5000, -5000, 5000], shape, z=-500)
    data = prism.gz(x, y, z, model)
    with pytest.warns(UserWarning):
        up = transform.upcontinue(x, y, data, shape, height=0)
    with pytest.warns(UserWarning):
        up = transform.upcontinue(x, y, data, shape, height=-100)
@rafaelmds

This comment has been minimized.

Member

rafaelmds commented Nov 25, 2016

@leouieda Sure! I thought that was necessary but I wasn't sure about it.

@leouieda

This comment has been minimized.

Member

leouieda commented Nov 25, 2016

No problem. This is the part that always gets newcomers.

Could you also please change the warning message to "Using 'height' <= 0 means downward continuation, which is known to be unstable." That might be more informative.

@leouieda

This comment has been minimized.

Member

leouieda commented Nov 25, 2016

After that, you can add the changelog entry for this PR in doc/changelog.rst and you're done 👍

@leouieda leouieda merged commit 061fb44 into fatiando:master Nov 25, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 73.61%
Details
@leouieda

This comment has been minimized.

Member

leouieda commented Nov 25, 2016

@rafaelmds merged! Thanks

@rafaelmds rafaelmds deleted the rafaelmds:upward-continuation-warning branch Nov 25, 2016

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