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 tilt transformation #486

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

leomiquelutti
Copy link
Contributor

@leomiquelutti leomiquelutti commented Mar 29, 2024

Changes proposed:

  • added tilt function to _transformations.py
  • added it to the index.rst
  • created an example in tilt.py
  • added it to __init__.py
  • added the class TestTilt in test_transformations.py

Some points/questions:

  • The TestTilt::test_against_synthetic is failing and I don't know why.
  • Maybe the plots in tilt.py require a title. I added but had a hard time with pyGMT, so I left them commented.
  • I think we should merge the examples, maybe all in one .py file, comparing the transformations from TMI to the ones from RTP TMI. But that should be done in another PR.
  • Do I add it to transformations.rst?

Can you please help me @santisoler.

* added `tilt` function to `_transformations.py`
* added it to the `index.rst`
* created an example in `tilt.py`
* added it to `__init__.py`
* added the class `TestTilt` in `test_transformations.py`; however, the test is failling.
For some reason that I don't know the test of tilt passes when you change the sinal of g_z in `TestTilt:test_against_synthetic` in `test_transformations.py`
Copy link
Member

@leouieda leouieda left a comment

Choose a reason for hiding this comment

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

Thanks for this @leomiquelutti! Made a few comments but nothing major. Good work!

harmonica/_transformations.py Show resolved Hide resolved
harmonica/_transformations.py Show resolved Hide resolved
harmonica/_transformations.py Show resolved Hide resolved
harmonica/_transformations.py Outdated Show resolved Hide resolved
harmonica/_transformations.py Show resolved Hide resolved
harmonica/tests/test_transformations.py Show resolved Hide resolved
harmonica/_transformations.py Show resolved Hide resolved
Co-authored-by: Leonardo Uieda <leo@uieda.com>
@leomiquelutti
Copy link
Contributor Author

Do you know why the test for macOS for python 3.8 is failing and/or how to solve it, @leouieda?

@santisoler
Copy link
Member

It was due to a failure when trying to push the coverage report to Codecov. I reran it and now it's passing. Nothing wrong with your code ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants