-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Adds ability to plot Pfsspy field lines #12
Conversation
What is the output from the new example? |
from sunkit_pyvista import SunpyPlotter | ||
|
||
############################################################################### | ||
# We will firstly use an AIA 193 image from the sunpy sample data as the base image. |
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.
This will be an issue if the AIA map and the GONG map were not taken at the same time. I will see if I can add a GONG map to the pfsspy sample data that is taken at the same observation date as the AIA map. Lets not block this PR on this point, but we should fix it before releasing the package!
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.
Oh okay, yes
sunkit_pyvista/plotter.py
Outdated
tracer = tracing.PythonTracer() | ||
input_ = pfsspy.Input(gong_map, nrho, rss) | ||
output_ = pfsspy.pfss(input_) | ||
|
||
seeds = SkyCoord(lon, lat, radius*R_sun, | ||
frame=gong_map.coordinate_frame) | ||
|
||
field_lines = tracer.trace(seeds, output_) |
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.
These lines should all go in the example script, and the plot_field_lines
method should just take field_lines
and **kwargs
as input. That way, the user has access to the PFSS solution if they want to trace different lines through it or do other things that aren't plotting the lines.
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.
yeah this makes more sense, I'll update the PR
Co-authored-by: David Stansby <dstansby@gmail.com>
This looks great! |
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
A rough draft on adding functionality to plot field lines.
Most of the field creation of the lon, lat and and radius values are done in the example file and I'm not sure this is how we want it structured here.
Should we allow
plot_field_lines
method to accept Skycoord only as an argument and directly plot the field lines instead of creating the Skycoord with thelon
,lat
andr
values being passed to SkyCood?