spectral_density equivalency created without factor fails equality test with one created with a factor

[braingram]

Description

#16343 deprecated factor and mentions passing a Quantity to wave instead of providing a factor. If 2 equivalencies are created, one with factor one with a Quantity the 2 equivalencies will fail an equality check.

This is likely in-part due to the kwargs comparison used in Equivalency.__eq__:

astropy/astropy/units/equivalencies.py

Line 71 in 5078f0b

and self.kwargs == other.kwargs

However comparing the equivalencies list of the 2 equivalencies also fail due to the local functions defined in spectral_density:

astropy/astropy/units/equivalencies.py

Lines 222 to 223 in 5078f0b

	def f_la_to_f_nu(x):
	return x * (wav.to_value(si.AA, spectral()) ** 2 / c_Aps)

Expected behavior

No response

How to Reproduce

Based off the example in the docs:

e0 = u.spectral_density(3500 * u.AA)
e1 = u.spectral_density(u.AA, factor=3500)

# conversion is the same
assert (1.5 * u.Jy).to(u.photon / u.cm**2, equivalencies=e0) == (1.5 * u.Jy).to(u.photon / u.cm**2, equivalencies=e1)

# equality fails
assert e0 != e1

Versions

platform
--------
platform.platform() = 'macOS-12.6-arm64-arm-64bit'
platform.version() = 'Darwin Kernel Version 21.6.0: Mon Aug 22 20:19:52 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T6000'
platform.python_version() = '3.10.6'

packages
--------
astropy              7.0.0.dev120+g5078f0b8e
numpy                1.25.1
scipy                1.11.1
matplotlib           --
pandas               --
pyerfa               2.0.1.4

[pllim]

I don't think this is worth fixing. factor was discouraged for a long time. Downstream should stop using it.

[braingram]

This is causing an issue for asdf-astropy as the keyword arguments are stored in the file (so that the equivalency will "round-trip" producing an "equal" equivalency when read back from the file).

This means that ASDF files contain "factor" if they contain an equivalency that was created with factor. It should be possible to correct for this when the file is read (if it contains a "factor" multiply it with "wav" before creating the equivalency). However this produces an equivalency that is not "equal" given the implementation of Equivalency.__eq__ as noted in this issue.

We can certainly work around this in asdf-astropy but I figured I would open this issue as this looked like a bug to me.

[pllim]

It is very unlikely that we will have two different implementations of the same equivalency ever again, so I still don't think this is worth fixing but maybe @mhvk, @nstarman, or @eerovaher have different opinions.

[mhvk]

Agreed with @pllim that this does not quite seem worth fixing. However, since we only properly deprecated this for 7.0, I think it is OK to have a solution in astropy if that turns out to be much easier to implement than in asdf - we just have to be sure it is marked properly so that we would remove the work-around when support for factor is removed altogether.

[github-actions]

Hi humans 👋 - this issue was labeled as Close? approximately 14 hours ago. If you think this issue should not be closed, a maintainer should remove the Close? label - otherwise, I will close this issue in 7 days.

If you believe I commented on this issue incorrectly, please report this here

[github-actions]

I'm going to close this issue as per my previous message, but if you feel that this issue should stay open, then feel free to re-open and remove the Close? label.

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here