Skip to content

Add changelog entries from PR #100 - #104

Merged
bsipocz merged 2 commits into
astropy:masterfrom
sargas:add-distortionaware-to-changes
May 9, 2017
Merged

bsipocz merged 2 commits into
astropy:masterfrom
sargas:add-distortionaware-to-changes

Conversation

@sargas

@sargas sargas commented Sep 5, 2016

Copy link
Copy Markdown
Contributor

PR to note the changes that #97 would have done, and warn about changing science results.

@cdeil cdeil added this to the 2.0 milestone Sep 5, 2016
@cdeil

cdeil commented Sep 5, 2016

Copy link
Copy Markdown
Member

👍

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 63.13% when pulling d594f17 on sargas:add-distortionaware-to-changes into 3cef819 on astropy:master.

@sargas sargas changed the title Add to changelog about switch to distortion-aware pixel conversions Add changelog entries from PR #100 Sep 5, 2016
@sargas

sargas commented Sep 5, 2016

Copy link
Copy Markdown
Contributor Author

I managed to change the API in another way that's undocumented: WCS objects are not sufficient for conversion to image coordinates.

This is because angles are defined based on the rotation of the north axis onto the pixel plane, as measured by the center of the FITS image. It would make more sense to me to make it at the center of the shape in question, or even based on CRPIX[12], but this is how DS9 does it. Added the documentation update to this PR.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 63.13% when pulling b62cc03 on sargas:add-distortionaware-to-changes into 3cef819 on astropy:master.

@cdeil

cdeil commented Apr 12, 2017

Copy link
Copy Markdown
Member

@sargas - Why wasn't this merged? Is it ready to go? (if yes, just go ahead and merge, please)

Comment thread docs/getting_started.rst
w = WCS(f[0].header)
w_im = w.celestial
r2 = pyregion.parse(region_string).as_imagecoord(w_im)
`ShapeList.as_imagecoord <pyregion.ShapeList.as_imagecoord>` will use the

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.

I'm not sure this will be properly rendered, but suggest to merge now and follow-up with a fix if needed.

@bsipocz

bsipocz commented May 9, 2017

Copy link
Copy Markdown
Member

I take the blame, and push the button now...

@bsipocz
bsipocz merged commit 40716a9 into astropy:master May 9, 2017
@cdeil

cdeil commented May 9, 2017

Copy link
Copy Markdown
Member

@sargas and @bsipocz - Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants