Make GOES event location variable names more explicit: lon/lat

Open
#22 12 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
58/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Quiet
Tech stack
python
Domain
data

Research direction

Start in sunpy/instr/goes at get_goes_event_list and inspect how the event_coord1 and event_coord2 values are exposed. Search the repository for callers, tests, and references to goes_location to identify the affected interface. Done means the returned names distinguish longitude from latitude and the relevant uses and tests reflect the change.

Written by the indexing model from the issue text.

Description

I've just done a bunch of digging around and determined that when using get_goes_event_list, the first array coordinate is longitude and the second is latitude. This is the same as what's returned when using IDL solarsoft rd_gev (which you can drill down several levels to find gbo_struct [code], where a comment defines these two location values). This is the opposite of what you see in the GOES event lists on the NOAA web interfaces (e.g., here) where location is reported as latitude, longitude.
I noticed this when looking at several years of flare event data and checking the minmax on the two locations. The first location ranges from [-90, +90] and the second (in the timeframe I was looking) from [-32, +56]. I had serious doubts that the GOES event list had anything reported from the poles.

So my suggested change is to make variable names more explicit so people in the future don't make mistakes, or have uncertainty if they're new to solar physics, or have to go through the exercise I just did. In sunpy/instr/goes there is a function called get_goes_event_list. In there, it defines location just as it is in IDL solarsoft:
'goes_location': (r['event_coord1'], r['event_coord2'])

I suggest this be changed to goes_longitude and goes_latitude

I'm happy to make this change (or a similar one based on feedback) in a PR but I'm not sure what other code it may break.

Dominant language
Python
Stars
15
Forks
20
Avg merge
9h 48m
Merged PRs (30d)
5

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from sunpy/sunkit-instruments

All issues in sunpy/sunkit-instruments

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.