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

fixed bug for highest flux at start or end of spectrum #377

Merged
merged 2 commits into from
Nov 3, 2018

Conversation

brechmos-stsci
Copy link

There was an issue with a spectrum where the highest flux was at the beginning of the spectrum. This meant the left was empty which caused some issues. This should now be fixed in this code.

Fixes #374

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

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

LGTM except for one very minor thing - so approving, although @brechmos-stsci might want to make the small adjustment before we merge this.

@@ -460,6 +460,23 @@ def test_fwhm():
assert quantity_allclose(result, expected, atol=0.01*u.GHz)


# Highest point at the first point
frequencies = np.linspace(1, 10, 100) * u.um
Copy link
Member

Choose a reason for hiding this comment

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

(minor) this isn't actually a frequency - it's a wavelength, so the variable name is a bit confusing

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

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

Test failures are a red herring (#379), so merging this now!

@eteq eteq merged commit 0fbcb21 into astropy:master Nov 3, 2018
@brechmos-stsci brechmos-stsci deleted the i374-fwhm-bug branch December 5, 2018 19:06
SaOgaz pushed a commit that referenced this pull request Mar 25, 2019
Fix for checking file existence if it is a comma separated list.
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.

argmax Bug in _compute_single_fwhm
2 participants