-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
spectral_density
equivalency created without factor
fails equality test with one created with a factor
#16435
Comments
I don't think this is worth fixing. |
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 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. |
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. |
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 |
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 |
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 |
Description
#16343 deprecated
factor
and mentions passing aQuantity
towave
instead of providing afactor
. If 2 equivalencies are created, one withfactor
one with aQuantity
the 2 equivalencies will fail an equality check.This is likely in-part due to the
kwargs
comparison used inEquivalency.__eq__
:astropy/astropy/units/equivalencies.py
Line 71 in 5078f0b
However comparing the
equivalencies
list of the 2 equivalencies also fail due to the local functions defined inspectral_density
:astropy/astropy/units/equivalencies.py
Lines 222 to 223 in 5078f0b
Expected behavior
No response
How to Reproduce
Based off the example in the docs:
Versions
The text was updated successfully, but these errors were encountered: