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

Added in spectrum_from_model #338

Merged

Conversation

brechmos-stsci
Copy link

This is a bit of a helper/utility method that creates a Spectrum1D object from a model given an input spectrum. The output Spectrum1D object will have all the same values as the input Spectrum1D except the flux will be based on the input model.

Fixes #337

@brechmos-stsci brechmos-stsci added this to the v0.4.0 milestone Sep 20, 2018
@brechmos-stsci brechmos-stsci self-assigned this Sep 20, 2018
@@ -152,3 +152,42 @@ def excise_region(spectrum, region, exciser=linear_exciser):
#

return exciser(spectrum, region)


def spectrum_from_model(model_input, spectrum):
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be added to __all__ if it's intended for the public API

Copy link
Author

Choose a reason for hiding this comment

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

👍

The input model or compound model from which flux is calculated.

spectrum : `~specutils.Spectrum1D`
The `~specutils.Spectrum1D` object to which the smoothing will be applied.
Copy link
Member

Choose a reason for hiding this comment

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

Copy-and-paste error? Probably should be "The ~specutils.Spectrum1D object to use as the model template" or something like that.

Copy link
Author

Choose a reason for hiding this comment

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

👍


return Spectrum1D(flux=flux,
spectral_axis=spectrum.spectral_axis,
uncertainty=spectrum.uncertainty,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to copy over the uncertainty here - the uncertainty is not the same for the model (in one interpretation it's zeros, and in another it's undefined).

(Should also note this in the docstring)

Copy link
Author

Choose a reason for hiding this comment

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

👍

@eteq
Copy link
Member

eteq commented Sep 20, 2018

A bit more context: this provides an alternative solution to #333 from having the model itself know what to do. We may want this anyway, but its proximate reason is that.

@eteq
Copy link
Member

eteq commented Sep 21, 2018

Since this isn't critical functionality and we're very close, I'm pushing this to 0.5

@eteq eteq modified the milestones: v0.4.0, v0.5.0 Sep 21, 2018
@nmearl
Copy link
Contributor

nmearl commented Jul 2, 2019

@brechmos Can you rebase this?

@brechmos-stsci brechmos-stsci force-pushed the fix-337-create-spectrum-from-model branch from a12efa0 to d9f96c0 Compare July 8, 2019 13:41
@nmearl nmearl merged commit 05251df into astropy:master Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create utility method to create a spectrum from a model and spectrum
4 participants