-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
fov table joins added #143
base: main
Are you sure you want to change the base?
Conversation
Can we make FOV opt in? |
Can you explain what you mean by this? |
I've tried to add a functionality, FOV can be passed in the search query with "earth" and "solar" values which would give coordinates with respect to that, if there is no FOV in search query it returns the normal table, if the user passes FOV in search query, depending on the value of it being solar or earth, the user will get all four coordinates. |
By making it a new attribute a user can add to the query instead of being done all the time. |
We should create the example as well. Might as well go all in. |
Alrighty! |
Looks like the doc test is failing, I'll fix it. |
examples/FOV.py
Outdated
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.
The file should be renamed to be something slightly more descriptive.
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, I'll make all the necessary changes. I just pushed my initial code before leaving for college.
examples/FOV.py
Outdated
import sunpy_soar # NOQA: F401 | ||
|
||
##################################################### | ||
# We shall start with constructing a search query. |
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.
I would explain what query you are constructing and why
examples/FOV.py
Outdated
# Create a blank map. | ||
|
||
data = np.full((10, 10), np.nan) | ||
skycoord = SkyCoord(0 * u.arcsec, 0 * u.arcsec, obstime="2013-10-28", observer="earth", frame=frames.Helioprojective) |
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.
Does the obstime match the search?
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.
I'll update it.
examples/FOV.py
Outdated
blank_map.draw_grid(axes=ax, color="k") | ||
|
||
# Plot the FOVs | ||
ax.plot_coord(earth_fov_corners, color="blue", linestyle="-", label="Earth FOV") |
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.
Can we not use quadrangle?
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, I'll use that.
Co-authored-by: Nabil Freij <[email protected]>
bottom_left_tx = min(fov_earth_bot_left_tx, fov_earth_top_right_tx) | ||
bottom_left_ty = min(fov_earth_bot_left_ty, fov_earth_top_right_ty) | ||
top_right_tx = max(fov_earth_bot_left_tx, fov_earth_top_right_tx) | ||
top_right_ty = max(fov_earth_bot_left_ty, fov_earth_top_right_ty) |
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.
Why do we need to do this?
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.
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.
What are the values returned from all of fov_earth
variables.
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.
"Earth FOV coordinates: {fov_earth_bot_left_tx}, {fov_earth_bot_left_ty}, {fov_earth_top_right_tx}, {fov_earth_top_right_ty}"
Values:
Earth FOV coordinates: -567.5620750957169 , -374.20399399515225 , -833.5243687650654 , -74.82417116247366
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.
That looks like the wrong values to for each pair.
Can you check we got the values from the SOAR correctly?
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.
I think these are the values I get from the SOAR, maybe I am missing something in the plotting?
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.
The numbers for fov_earth_bot_left_tx and fov_earth_top_right_tx are reversed.
I assume that's on our side?
What is strange is that those top right values look like the values of the left edge.
Can you double check the example from the issue to make sure we are getting the correct values?
Something is up but I dont know what exactly.
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.
Okay, will look into it.
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.
As we have just discussed, this is probably because on 2022-10-13T12 (date of the example code) the Earth-Sun-SO angle was 109°, so the SO FOV was on the far side of the Sun seen from Earth. It appears that SOAR converts the bottom left and top right (θx, θy) helioprojective coordinates of the corners of the field of view seen from SO (assuming 0° roll) to Earth view, dropping the 3rd coordinate and not taking into account what is visible from Earth or not.
To implement functionality for plotting curves using Field of View (FoV) values available in SOAR, we needed to first add support for these tables in sunpy-soar. This PR introduces that necessary support for FoV tables.