Bugfix/filter sed conversion#80
Conversation
| @@ -292,10 +292,49 @@ def interpolate(self, wavelength=None): | |||
| return self.response | |||
|
|
|||
| def _check_spectra(self, spectra, default_unit=u.Lsun / u.angstrom / u.cm**2): | |||
There was a problem hiding this comment.
Perhaps _ensure_f_lambda, _ensure_f_lambda_units, or ensure_spectra_units would be more descriptive... I think I lean towards the second (perhaps renaming spectra as f_lambda?)
There was a problem hiding this comment.
This is a private method. I don't know to what extent we need to be that careful given the docstrings... I don't understand the comment about the unit argument.
|
|
||
| Notes | ||
| ----- | ||
| - Requires `self.wavelength` to be defined as an astropy Quantity with |
There was a problem hiding this comment.
Looks like we should implement _ensure_wavelength_units
There was a problem hiding this comment.
I am not sure, self.wavelength cannot be anything else but a quantity with length dimensions or None. I think what I am missing here is checking whether the attribute is None.
|
@paranoya lmk if you think there's something important missing, otherwise we can merge (the fix was for a student) |
| spectra = u.Quantity(spectra, default_unit) | ||
| # Check flux density units | ||
| if spectra.unit.is_equivalent(u.W / (u.m**2 * u.m)): | ||
| return spectra |
There was a problem hiding this comment.
Actually, what about the following helper function?
def ensure_unit(x, unit, just_equivalent=True, equivalence=None):
if not isinstance(x, u.Quantity):
return x << unit
if x.unit.is_equivalent(unit):
if just_equivalent:
return x
else:
return x.to(unit)
else:
return x.to(unit, equivalencies=equivalence)and then
ensure_unit(wavelength, u.AA)
ensure_unit(spectra, u.Lsun/u.angstrom, equivalence=u.spectral_density(wavelength))There was a problem hiding this comment.
I modified the check_unit method to also accept equivalencies, either taking arguments (spectral_density) or not ( spectral). The main difference is that the equivalency is used only when the input quantity is not equivalent to the target unit. This reduces the number of calls when using SSP data, which by default is in Flam.
I see that CI testing is ongoing. Please go ahead once this is done @PabloCorcho. |
2 participants
Filternow accepts flux density per wavelength and frequency unit. This avoids a lot of troubles for users that get confused with the units.