Separate VEGAMAG and OBMAG

[pllim]

Description

This pull request is to fix #327

This is breaking change! Unit string will change from, say, 'VEGAMAG' to 'mag(VEGA)'. Also the behavior also changes because it is now a magnitude object, not just "plain Quantity."

TODO

• Make tests pass.
• Test stsynphot against this PR branch. See DO NOT MERGE: Test against synphot_refactor PR 331 stsynphot_refactor#162
• Need blessing from ETC. cc @vglaidler @ariedel @cdsontag
  • Contacted them on 2022-05-17
  • Response from Adric: I tried it on some of our regression tests that use vegamags and abmags (we have none that use OBmags). Our multimission/normalization and jwst/workbooks test suites all passed without complaint. Pandeia is compatible with your change... I think the PR is OK to merge. It passes all the relevant tests. (2022-05-23)

Blocked by (SOLVED)

@mhvk , I am trying to follow your advice at astropy/astropy#13158 (comment) but I am unable to get the new magnitude definitions to work with existing unit conversion equivalencies. What am I missing?

synphot_refactor/synphot/units.py

Lines 244 to 245 in bd22960

	flux_photlam = _convert_flux(
	wavelengths, fluxes, PHOTLAM, **kwargs)

synphot_refactor/synphot/units.py

Lines 291 to 293 in bd22960

	out_flux = fluxes.to(
	out_flux_unit,
	equivalencies=spectral_density_count(wavelengths, area))

synphot_refactor/synphot/units.py

Line 128 in bd22960

def spectral_density_count(wav, area):

.../synphot/units.py in _convert_flux(wavelengths, fluxes, out_flux_unit, area, vegaspec)
    289             area = area * AREA
    290
--> 291         out_flux = fluxes.to(
    292             out_flux_unit,
    293             equivalencies=spectral_density_count(wavelengths, area))

.../astropy/units/quantity.py in to(self, unit, equivalencies, copy)
    844             # Avoid using to_value to ensure that we make a copy. We also
    845             # don't want to slow down this method (esp. the scalar case).
--> 846             value = self._to_value(unit, equivalencies)
    847         else:
    848             # to_value only copies if necessary

.../astropy/units/quantity.py in _to_value(self, unit, equivalencies)
    798         if not self.dtype.names or isinstance(self.unit, StructuredUnit):
    799             # Standard path, let unit to do work.
--> 800             return self.unit.to(unit, self.view(np.ndarray),
    801                                 equivalencies=equivalencies)
    802

.../astropy/units/function/core.py in to(self, other, value, equivalencies)
    255             try:
    256                 # when other is not a function unit
--> 257                 return self.physical_unit.to(other, self.to_physical(value),
    258                                              equivalencies)
    259             except UnitConversionError as e:

.../astropy/units/core.py in to(self, other, value, equivalencies)
   1128             return UNITY
   1129         else:
-> 1130             return self._get_converter(Unit(other),
   1131                                        equivalencies=equivalencies)(value)
   1132

.../astropy/units/core.py in _get_converter(self, other, equivalencies)
   1044         # if that doesn't work, maybe we can do it with equivalencies?
   1045         try:
-> 1046             return self._apply_equivalencies(
   1047                 self, other, self._normalize_equivalencies(equivalencies))
   1048         except UnitsError as exc:

.../astropy/units/core.py in _apply_equivalencies(self, unit, other, equivalencies)
   1002                     pass
   1003                 try:
-> 1004                     scale1 = tunit._to(unit)
   1005                     scale2 = funit._to(other)
   1006                     return make_converter(scale1, b, scale2)

AttributeError: 'MagUnit' object has no attribute '_to'

[codecov]

Codecov Report

Merging #331 (dc076f7) into master (bd22960) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
- Coverage   94.31%   94.29%   -0.02%     
==========================================
  Files          14       14              
  Lines        2005     1999       -6     
==========================================
- Hits         1891     1885       -6     
  Misses        114      114              

Impacted Files	Coverage Δ
synphot/observation.py	99.06% <100.00%> (ø)
synphot/units.py	91.72% <100.00%> (-0.36%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd22960...dc076f7. Read the comment docs.

[mhvk]

OK, I think I can see why spectral_density_count equivalency would not work for mag(OB). What's needed is a conversion to OB, and let the Magnitude take care of the logs. So, something like:

PHOTLAM = u.def_unit(
    'photlam', u.photon / (u.cm**2 * u.s * u.AA),
    format={'generic': 'PHOTLAM', 'console': 'PHOTLAM'})
_ob = u.def_unit('OB')
OBMAG = u.mag(_ob)
factor = 100  # calculate as in equivalency
equiv = [(PHOTLAM, OBMAG.physical_unit, lambda x: x*factor, lambda x: x/factor)]
(10*OBMAG).to(PHOTLAM, equivalencies=equiv)
# <Quantity 1.e-06 PHOTLAM>

From a quick look, it seems that in your spectral_density_count equivalency, the conversion routines now become identical for counts and OBMAG.

[pllim]

Thanks, @mhvk ! That is very helpful. 😸