Skip to content
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

Fix vector directions when overlaid on image #2276

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Carifio24
Copy link
Member

When a scatter layer is overlaid on an image, vectors will have the wrong orientation if one (or both) of the axes have different pixel and world coordinate orientations. To fix this issue, this PR looks at the signs of the relevant entry of the cdelt coordinate increments. If they're opposite, it negates the corresponding vector components. The relation between x/y attributes and the wcs coordinate indices is the same one used here.

I would be curious to know if @astrofrog or @catherinezucker (or anyone who's knowledgeable about WCS) has any additional thoughts/suggestions.

…ve axis orientations and adjust accordingly.
@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #2276 (577cd83) into main (2d64631) will decrease coverage by 0.02%.
The diff coverage is 58.62%.

❗ Current head 577cd83 differs from pull request most recent head 9bbdf09. Consider uploading reports for the commit 9bbdf09 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2276      +/-   ##
==========================================
- Coverage   88.07%   88.04%   -0.03%     
==========================================
  Files         247      247              
  Lines       23279    23302      +23     
==========================================
+ Hits        20502    20517      +15     
- Misses       2777     2785       +8     
Impacted Files Coverage Δ
glue/viewers/scatter/layer_artist.py 95.10% <11.11%> (-2.11%) ⬇️
glue/conftest.py 63.75% <78.94%> (+10.71%) ⬆️
glue/core/state.py 92.12% <100.00%> (ø)
glue/_mpl_backend.py 80.00% <0.00%> (-8.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d64631...9bbdf09. Read the comment docs.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

I'm not sure if using CDELT is sufficient, as there might be cases where an image is rotated using CROTA2 or a CD matrix, so then I'm not sure if the logic here still holds.

I'm trying to wrap my head around the use case here - what kind of table columns are you using to define the vector? Are they pixel or world coordinates?

@Carifio24
Copy link
Member Author

This PR definitely fell off of my radar. @astrofrog I had used the get_cdelt() method since the astropy docs say that it should work for CD or CROTA as well. I don't have a good set of examples of these on hand, but will certainly try this out if someone has any.

As far as defining the vectors, it looks like numerical components are what are allowed for the vector attributes of a scatter layer.

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.

3 participants