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
Remove an erroneous special case from search_around_3d()
#16280
Conversation
`search_around_3d()` expects the search radius to be a length and it expects its input coordinates to have physical distances (i.e. not with the unit `dimensionless_unscaled`). If that is not the case a `UnitConversionError` is raised, unless at least one of the coords is empty. The new tests reveal the erroneous special case.
The function now raises the expected `UnitConversionError` if the units of the distances of the input coordinates and the search limit do not agree even if one of the input coordinates is empty.
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agreed! And good in general to get rid of special cases; it just causes inconsistencies like you found here.
I think the readthedocs failure has been solved, but perhaps that is not quite worth running all CI again for - so, maybe @pllim can wave the magic merge wand? |
Sorry for the delay. I was away watching a rock passing in front of a ball of gas. 😆 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Wasn't it just spectacular?!! |
…rch_around_3d()` Cherry picked from commit 9dff116
…rch_around_3d()` Cherry picked from commit 9dff116
Backport PR #16280 on branch v6.1.x (Remove an erroneous special case from search_around_3d())
Yes!!! |
Description
In
search_around_3d()
ifcoord1.distance
,coord2.distance
anddistlimit
have incompatible units (e.g.distlimit
is a length, butcoords1
orcoords2
has a distance withdimensionless_unscaled
) then it will raise anUnitConversionError
, unless at least one of the input coordinates is empty. I cannot think of a good reason why such a special case should exist, and in some cases this behavior can be the cause of aUnitConversionError
somewhere else in the code, depending on what the unit ofcoords1.distance
is.I expect the automatic backport to fail because of recent changes to the tests.