Add GONG H-Alpha map source #7451

samaloney · 2024-02-16T14:44:09Z

PR Description

Closes #6656 by adding a GONG H-alpha map source.

TODO:

Add changelog
Update docs and add links to source material
Add tests
Use Earth lat, lon header to set observatory location
Better default plot settings

Image from GONG site

From sunpy with this PR

sunpy/map/sources/gong.py

samaloney · 2024-02-16T17:17:07Z

Because of the asdf issue with the .fz extension you have to open these maps with the well hidden filetype keyword Map('/path/to/file/20240216000002Bh.fits.fz', filetype='fits')

nabobalis · 2024-02-16T18:59:17Z

We need to fix that bug, and milestone it for 6.0

sunpy/map/sources/gong.py

nabobalis · 2024-02-21T16:13:13Z

sunpy/map/sources/gong.py

+
+    @property
+    def observer_coordinate(self):
+        return SkyCoord(self.earth_location.itrs).heliographic_stonyhurst


Do we have a test for this?

No but I can add one but there is a test for self.earth_location and if this were to break it would be in SkyCoord or EarthLocation

sunpy/map/sources/gong.py

samaloney · 2024-03-02T17:58:31Z

I think the test fail is related to this issue astropy/astropy/issues/16128

samaloney · 2024-03-04T21:50:47Z

Yea so if I extract the test command and run in a terminal it works I guess it could be something to do with remote_data but I tried adding that marker and it didn't work either?

import astropy.units as u
from numpy.testing import assert_equal
from sunpy.data.test import get_dummy_map_from_header, get_test_filepath

gm = get_dummy_map_from_header(get_test_filepath('gong_halpha.header'))
assert_equal(gm.observer_coordinate.data.xyz, 
             [146712246479.363, -5563586.169750214, -17605285536.73928] * u.m)

wtbarnes · 2024-03-07T17:55:19Z

Yea so if I extract the test command and run in a terminal it works I guess it could be something to do with remote_data but I tried adding that marker and it didn't work either?
import astropy.units as u
from numpy.testing import assert_equal
from sunpy.data.test import get_dummy_map_from_header, get_test_filepath

gm = get_dummy_map_from_header(get_test_filepath('gong_halpha.header'))
assert_equal(gm.observer_coordinate.data.xyz, 
             [146712246479.363, -5563586.169750214, -17605285536.73928] * u.m)

I'm able to reproduce this test fail locally. The reason is because the last date in the IERS table that astropy is grabbing is 1/12/2024 and the date of this observation is 2/16/2024. Thus, its throwing a warning that you're beyond the last date in the IERS table and your accuracy may be degraded.

samaloney · 2024-03-07T18:05:38Z

Yea but is that specific to test runs e.g. strict remote data runner as if I run it from local shell it works?

    ~/Projects/sunpy-dev    feat-gong-halpha *3 ?1                                                                                                                                                                                                                                                                                                 ✔  sunpy-dev 
    ~/Projects/sunpy-dev    feat-gong-halpha *3 ?1  ipython                                                                                                                                                                                                                                                                                        ✔  sunpy-dev 
Python 3.11.7 (main, Dec  4 2023, 18:10:11) [Clang 15.0.0 (clang-1500.1.0.2.5)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.21.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import astropy.units as u
   ...: from numpy.testing import assert_equal
   ...: from sunpy.data.test import get_dummy_map_from_header, get_test_filepath
   ...:
   ...: gm = get_dummy_map_from_header(get_test_filepath('gong_halpha.header'))
   ...: assert_equal(gm.observer_coordinate.data.xyz,
   ...:              [146712246479.363, -5563586.169750214, -17605285536.73928] * u.m)

In [2]:

wtbarnes · 2024-03-07T18:05:41Z

Ok I think that should fix the test. Let me know what you think about that. I changed the assert_equal to an isclose with a tolerance of 1e-9. If we really need the assert_equal then we can interrogate what the actual differences are between the two.

wtbarnes · 2024-03-07T18:06:55Z

Yea but is that specific to test runs e.g. strict remote data runner as if I run it from local shell it works?

    ~/Projects/sunpy-dev    feat-gong-halpha *3 ?1                                                                                                                                                                                                                                                                                                 ✔  sunpy-dev 
    ~/Projects/sunpy-dev    feat-gong-halpha *3 ?1  ipython                                                                                                                                                                                                                                                                                        ✔  sunpy-dev 
Python 3.11.7 (main, Dec  4 2023, 18:10:11) [Clang 15.0.0 (clang-1500.1.0.2.5)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.21.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import astropy.units as u
   ...: from numpy.testing import assert_equal
   ...: from sunpy.data.test import get_dummy_map_from_header, get_test_filepath
   ...:
   ...: gm = get_dummy_map_from_header(get_test_filepath('gong_halpha.header'))
   ...: assert_equal(gm.observer_coordinate.data.xyz,
   ...:              [146712246479.363, -5563586.169750214, -17605285536.73928] * u.m)

In [2]:

Either you're silencing the warning or using a different IERS table (that has a more recent date). Can you check which table you're using?

samaloney · 2024-03-07T18:13:19Z

Hum I cleared the downloaded cache in the process of trying to reproduce the errors so maybe that's related.

from astropy.utils import iers

iers.IERS_A_FILE
'finals2000A.all'

iers.IERS_B_FILE
'/Users/sm/.virtualenvs/sunpy-dev/lib/python3.11/site-packages/astropy/utils/iers/data/eopc04_IAU2000.62-now'

I could use an older H-alpha file for the tests before 2024/02/12?

wtbarnes · 2024-03-07T20:39:21Z

Maybe? Whatever I did also I did not fix the issue on the CI though did fix the fail for me locally. The most annoying combination.

Yeah, maybe just using an older file is the easiest solution. People will of course run into this if they use the newer files, but it's probably not a big deal in terms of accuracy or they'll have a more up-to-date IERS table.

wtbarnes · 2024-03-08T16:38:22Z

sunpy/map/sources/tests/test_gong_halpha_source.py

+def test_scale(gong_halpha):
+    """Tests the scale property of the GONGHalphaMap map."""
+    assert_equal(gong_halpha.scale.axis1, 1.0794939681708389 * (u.arcsec / u.pix))
+    assert_equal(gong_halpha.scale.axis1, 1.0794939681708389 * (u.arcsec / u.pix))


Should this be axis2?

wtbarnes · 2024-03-09T17:08:59Z

Just to cross-reference, the actual underlying in astropy is now being discussed here: astropy/astropy#13227 (comment)

The tl;dr is that astropy will ignore a just-downloaded IERS table if you have iers.conf.auto_download = False even though that table exists on your machine already.

I would suggest that if we don't want this PR held up by this bugfix, then we just choose a GONG header at an earlier date.

nabobalis · 2024-03-21T00:42:56Z

sunpy/map/sources/tests/test_gong_halpha_source.py

+    with pytest.warns(AstropyWarning, match='Tried to get polar motions for times after IERS data is valid.'):
+        with pytest.warns(AstropyWarning, match='times are outside of range covered by IERS table'):


Had to add an extra warning?

I broke oldest build do this.

> with pytest.warns(AstropyWarning, match='Tried to get polar motions for times after IERS data is valid.'): E Failed: DID NOT WARN. No warnings of type (<class 'astropy.utils.exceptions.AstropyWarning'>,) were emitted. The list of emitted warnings is: [].

Again I think we should just choose an older test header to avoid having to deal with this IERS behavior since an upstream fix is needed.

Even though the upstream fix (astropy/astropy#16187) will likely be merged imminently, it probably won't be backported, so it will be a long while before it is in all versions of astropy we support. The alternative to picking an older test header is to simply filter away warnings within the unit test.

That is, the unit test don't actually care if there's a warning or not, so catching/matching with pytest.warns() is unnecessary.

(And yes, there are two warnings, from two different parts of the astropy codebase.)

* Add tests and test data * Add observer_coord and update docs.

Co-authored-by: Will Barnes <[email protected]>

samaloney commented Feb 16, 2024

View reviewed changes

sunpy/map/sources/gong.py Show resolved Hide resolved

samaloney force-pushed the feat-gong-halpha branch from 3427043 to e709fc4 Compare February 16, 2024 16:40

nabobalis added the map Affects the map submodule label Feb 16, 2024

samaloney force-pushed the feat-gong-halpha branch from e709fc4 to ea6e52a Compare February 16, 2024 17:13

samaloney marked this pull request as ready for review February 16, 2024 17:14

samaloney requested a review from a team as a code owner February 16, 2024 17:14

nabobalis reviewed Feb 16, 2024

View reviewed changes

sunpy/map/sources/gong.py Outdated Show resolved Hide resolved

nabobalis reviewed Feb 16, 2024

View reviewed changes

sunpy/map/sources/gong.py Outdated Show resolved Hide resolved

samaloney force-pushed the feat-gong-halpha branch from ea6e52a to c9bbc8c Compare February 21, 2024 12:31

nabobalis reviewed Feb 21, 2024

View reviewed changes

wtbarnes requested changes Feb 28, 2024

View reviewed changes

samaloney force-pushed the feat-gong-halpha branch from ec2f4a8 to 361ed29 Compare March 2, 2024 17:36

samaloney force-pushed the feat-gong-halpha branch from 361ed29 to 66c6455 Compare March 2, 2024 22:48

samaloney force-pushed the feat-gong-halpha branch from 66c6455 to 493707d Compare March 4, 2024 21:52

wtbarnes reviewed Mar 8, 2024

View reviewed changes

nabobalis force-pushed the feat-gong-halpha branch from 8cd494a to 03764b4 Compare March 21, 2024 00:42

nabobalis requested a review from wtbarnes March 21, 2024 00:42

nabobalis added the Needs Review Needs reviews before merge. label Mar 21, 2024

nabobalis reviewed Mar 21, 2024

View reviewed changes

nabobalis approved these changes Mar 21, 2024

View reviewed changes

nabobalis force-pushed the feat-gong-halpha branch from d4e591b to 2343d1b Compare March 21, 2024 03:24

samaloney and others added 5 commits March 20, 2024 20:25

Add GONG H-alpha map source

7d0ec2c

* Add tests and test data * Add observer_coord and update docs.

Review comments

bc28ebd

Co-authored-by: Will Barnes <[email protected]>

fix observer coordinate test

7c89f0f

Update tests

cecedb4

filter warning instead

30d1d4b

nabobalis force-pushed the feat-gong-halpha branch from 2343d1b to 30d1d4b Compare March 21, 2024 03:25

nabobalis added the No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) label Mar 21, 2024

alasdairwilson approved these changes Mar 21, 2024

View reviewed changes

wtbarnes approved these changes Mar 21, 2024

View reviewed changes

wtbarnes merged commit 4c17a50 into sunpy:main Mar 21, 2024
25 of 28 checks passed

samaloney deleted the feat-gong-halpha branch March 21, 2024 14:03

nabobalis removed the Needs Review Needs reviews before merge. label Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GONG H-Alpha map source #7451

Add GONG H-Alpha map source #7451

samaloney commented Feb 16, 2024 •

edited

Loading

samaloney commented Feb 16, 2024

nabobalis commented Feb 16, 2024

nabobalis Feb 21, 2024

samaloney Feb 21, 2024

samaloney commented Mar 2, 2024

samaloney commented Mar 4, 2024

wtbarnes commented Mar 7, 2024

samaloney commented Mar 7, 2024

wtbarnes commented Mar 7, 2024

wtbarnes commented Mar 7, 2024

samaloney commented Mar 7, 2024 •

edited

Loading

wtbarnes commented Mar 7, 2024 •

edited

Loading

wtbarnes Mar 8, 2024 •

edited

Loading

wtbarnes commented Mar 9, 2024 •

edited

Loading

nabobalis Mar 21, 2024

nabobalis Mar 21, 2024

wtbarnes Mar 21, 2024

ayshih Mar 21, 2024

ayshih Mar 21, 2024

ayshih Mar 21, 2024 •

edited

Loading

nabobalis Mar 21, 2024

		with pytest.warns(AstropyWarning, match='Tried to get polar motions for times after IERS data is valid.'):
		with pytest.warns(AstropyWarning, match='times are outside of range covered by IERS table'):

Add GONG H-Alpha map source #7451

Add GONG H-Alpha map source #7451

Conversation

samaloney commented Feb 16, 2024 • edited Loading

PR Description

Image from GONG site

From sunpy with this PR

samaloney commented Feb 16, 2024

nabobalis commented Feb 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samaloney commented Mar 2, 2024

samaloney commented Mar 4, 2024

wtbarnes commented Mar 7, 2024

samaloney commented Mar 7, 2024

wtbarnes commented Mar 7, 2024

wtbarnes commented Mar 7, 2024

samaloney commented Mar 7, 2024 • edited Loading

wtbarnes commented Mar 7, 2024 • edited Loading

wtbarnes Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

wtbarnes commented Mar 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayshih Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samaloney commented Feb 16, 2024 •

edited

Loading

samaloney commented Mar 7, 2024 •

edited

Loading

wtbarnes commented Mar 7, 2024 •

edited

Loading

wtbarnes Mar 8, 2024 •

edited

Loading

wtbarnes commented Mar 9, 2024 •

edited

Loading

ayshih Mar 21, 2024 •

edited

Loading