Use small angular offsets when determining compass properties #2867
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
astrofrog
requested review from
rosteen,
javerbukh,
pllim,
kecnry,
bmorris3,
haticekaratay,
cshanahan1 and
gibsongreen
as code owners
pllim
added
plugin
pllim
approved these changes
May 10, 2024
|
There are 2 failures that just need their results updated. |
pllim
force-pushed
the
small-delta-compass
branch
from
575d48f to
afef68a
Compare
pllim
reviewed
May 10, 2024
| @@ -98,18 +98,26 @@ def calc_compass(image_wcs, x, y, len_deg_e, len_deg_n): | |||
|
|
|||
|
|
|||
| def calc_compass_radius(image_wcs, x, y, radius_px): | |||
There was a problem hiding this comment.
@ejeschke , this function was actually from ginga:
Are you interested to have a similar patch over at ginga?
There was a problem hiding this comment.
That looks like a good change, so sure!
There was a problem hiding this comment.
OK please see ejeschke/ginga#1102 . Thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
While debugging an issue with GWCS coordinates with @bmorris3, I found that one issue is that if an image with a bounding box is used, and if we were to not reset the bounding box to
Noneas we do currently in jadviz, the compass properties will fail in some cases - this is because before this PR, the pixel scale along the N and E directions was determined by adding 1 deg to the RA and Dec, but this is too large as this may then fall outside the image and return NaN (with spacetelescope/gwcs#497).In any case, I think it makes sense to use small angular offsets when trying to do this calculation regardless of whether we decide to not disable the bounding box etc. I don't think there is any downside to using small offsets, and it will be more future-proof.
Change log entry
CHANGES.rst? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rstbefore merge. If no, maintainershould add a
no-changelog-entry-neededlabel.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
triviallabel.