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

Removing voigt_old functionality #139

Merged
merged 11 commits into from Dec 9, 2020
Merged

Conversation

chummels
Copy link
Collaborator

Trident has historically had two separate methods for numerically integrating the voigt profiles: one from some old IDL routines using chebyshev polonomials and one using a SciPy method. If you have scipy installed it would use the scipy method, otherwise it would use the IDL routine. At some point, the IDL routine stopped working. This PR removes that functionality and makes SciPy a hard dependency of Trident.

Note: We have a test to ensure that the two methods get the same values for a set of inputs in test_absorption_spectrum.py, which never seemed to fail. I want to investigate this first before merging to find out what went awry here.

@coveralls
Copy link

coveralls commented Jun 26, 2020

Pull Request Test Coverage Report for Build 1093

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.09%) to 75.824%

Files with Coverage Reduction New Missed Lines %
/home/circleci/trident/trident/absorption_spectrum/absorption_line.py 3 89.66%
Totals Coverage Status
Change from base Build 1066: -0.09%
Covered Lines: 1772
Relevant Lines: 2337

💛 - Coveralls

Copy link
Collaborator

@brittonsmith brittonsmith left a comment

Choose a reason for hiding this comment

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

Did you still want to investigate why the test was able to pass all this time? If not, I'm fine for this to be merged.

@@ -46,7 +46,7 @@
def test_voigt_profiles():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this test was meant to compare the two functions. We probably don't need to keep it.


def voigt_old(a, u):
Copy link
Collaborator

Choose a reason for hiding this comment

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

RIP voigt_old. You were a good function.

@chummels chummels merged commit 2cc7741 into trident-project:master Dec 9, 2020
@chummels chummels deleted the voigt_old branch December 9, 2020 02:57
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