Skip to content

Allow explicit negative distances in SphericalRepresentation - #11113

Merged
mhvk merged 3 commits into
astropy:masterfrom
mhvk:representation-allow-subclasses
Dec 3, 2020
Merged

mhvk merged 3 commits into
astropy:masterfrom
mhvk:representation-allow-subclasses

Conversation

@mhvk

@mhvk mhvk commented Dec 1, 2020 •

Copy link
Copy Markdown
Contributor

Fixes #11104, #8091

The problem was that any input Distance was turned into a Quantity instead of just passed through. It really should pass it through, as it is a subclass, so this PR allows it (which is what makes it an API change). But to fix that, a small change had to be made to Angle.__new__ to allow it to pass on keyword arguments like subok=True to Quantity.__new__.

Also, it turned out _combine_xyz was not safe for some operations with this change, but since that private function that was no longer needed anyway now our minimum numpy version is 1.17, so the first commit just turns that into a simple np.stack.

@mhvk mhvk added Bug coordinates API change PRs and issues that change an existing API, possibly requiring a deprecation period labels Dec 1, 2020
@mhvk mhvk added this to the v4.3 milestone Dec 1, 2020
@mhvk
mhvk requested review from adrn and astrojuanlu December 1, 2020 00:54

@astrojuanlu astrojuanlu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mhvk

mhvk commented Dec 3, 2020

Copy link
Copy Markdown
Contributor Author

Approved a few days ago, so I'll go ahead and merge.

@mhvk
mhvk merged commit cf31d42 into astropy:master Dec 3, 2020
@mhvk
mhvk deleted the representation-allow-subclasses branch December 3, 2020 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API change PRs and issues that change an existing API, possibly requiring a deprecation period Bug coordinates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot use negative distance in spherical representation despite error message

2 participants