increase min sunpy #63

nabobalis · 2024-07-23T23:27:07Z

This supersedes #48

Fixes Add towncrier #49
Renamed FortranTracer to PerformanceTracer
Deprecated FortranTracer
streamtracer is now a hard dep
Deprecated bunch of utils I think we don't need anymore.
Bunch of package updates
Trying to tidy up the example gallery
Adaptmap removed due to upstream support in sunpy

This turned into a large PR. I could split it out but i'm lazy.

sunkit_magex/pfss/tests/test_pfss.py

nabobalis · 2024-07-24T01:18:34Z

sunkit_magex/pfss/utils.py

-        hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))
-
-
-def load_adapt(adapt_path):


We dont have the same loader in sunpy but we can now load them directly. But only the first one.

I don't know enough about the data to know if the other ones are useful, but if there isn't a drop in replacement then maybe we shouldn't yank it?

Also deprecations please 👼

Cadair

Thanks @nabobalis

I think we need to be deprecating stuff, I made the first version 1.0.0 for a reason, I think stability is important here.

I will let @jgieseler and @STBadman chime in on things like load_adapt and fix_hmi_meta specifically, but I would like a review from at least one of them before we merge.

docs/installing.rst

pyproject.toml

sunkit_magex/pfss/tests/test_tracers.py

sunkit_magex/pfss/tracing.py

Cadair · 2024-07-24T08:38:14Z

sunkit_magex/pfss/utils.py

-        hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))
-
-
-def load_adapt(adapt_path):


I don't know enough about the data to know if the other ones are useful, but if there isn't a drop in replacement then maybe we shouldn't yank it?

Also deprecations please 👼

examples/using_sunkit_magex_pfss/overplotting_onto_aia.py

nabobalis · 2024-07-25T19:52:25Z

sunkit_magex/pfss/input.py

@@ -42,7 +42,6 @@ def __init__(self, br, nr, rss):
        sunkit_magex.pfss.utils.is_full_sun_synoptic_map(br, error=True)

        self._map_in = copy.deepcopy(br)
-        self.dtime = self.map.date


Not sure if we need this but it should be reference_date if we do.

I'd probably not remove this property from the Input class, so 👍 making it reference_date.

nabobalis · 2024-07-25T19:52:58Z

I need to patch sunpy 6.0.0 first and then this should pass.

nabobalis · 2024-09-17T14:35:50Z

But I'm a bit confused regarding fix_hmi_meta. You state that it's deprecated because this is now fixed within sunpy when you load a HMI file. But if I download a HMI file (with files = Fido.fetch(...)) and then open it with hmi_map = sunpy.map.Map(files[0]), all the things fix_hmi_meta should fix are still in it, like hmi_map.meta['cunit2'] = 'Sine Latitude'. Am I doing it wrong?

Oh in that case, I was very wrong and we should fix that in the map source.

The reason I said that was it was in the notes:

If you have sunpy > 2.1 installed, this function is not needed as sunpy
will automatically make these fixes.

But clearly I didn't check.

jgieseler · 2024-09-17T15:07:20Z

Yes, I saw that comment as well and was confused. That's why I was wondering if I do the loading in a wrong way.

nabobalis · 2024-09-17T15:11:07Z

Oh I am confused, sunpy doesn't change the meta anymore. If you access the attributes of the map, those should be correct now. I think .units?

jgieseler · 2024-09-17T15:24:04Z

Oh I am confused, sunpy doesn't change the meta anymore. If you access the attributes of the map, those should be correct now. I think .units?

Hm, I don't know. But I just checked and run some old code that uses fix_hmi_meta. I run it with applying fix_hmi_meta and also without (i.e., just loading the HMI file with sunpy). The resulting figures look the same, only in the latter case, some SunpyMetadataWarning are printed:

Missing metadata for solar radius: assuming the standard radius of the photosphere. [sunpy.map.mapbase]
2024-09-17 18:19:50 - sunpy - INFO: Missing metadata for solar radius: assuming the standard radius of the photosphere.
WARNING: SunpyMetadataWarning: Missing metadata for observer: assuming Earth-based observer.
For frame 'heliographic_stonyhurst' the following metadata is missing: hgln_obs,hglt_obs,dsun_obs
For frame 'heliographic_carrington' the following metadata is missing: crlt_obs,dsun_obs,crln_obs
 [sunpy.map.mapbase]
2024-09-17 18:19:50 - sunpy - WARNING: SunpyMetadataWarning: Missing metadata for observer: assuming Earth-based observer.
For frame 'heliographic_stonyhurst' the following metadata is missing: hgln_obs,hglt_obs,dsun_obs
For frame 'heliographic_carrington' the following metadata is missing: crlt_obs,dsun_obs,crln_obs

Fixed

nabobalis · 2024-11-11T22:53:40Z

Oh I am confused, sunpy doesn't change the meta anymore. If you access the attributes of the map, those should be correct now. I think .units?

Hm, I don't know. But I just checked and run some old code that uses fix_hmi_meta. I run it with applying fix_hmi_meta and also without (i.e., just loading the HMI file with sunpy). The resulting figures look the same, only in the latter case, some SunpyMetadataWarning are printed:

Missing metadata for solar radius: assuming the standard radius of the photosphere. [sunpy.map.mapbase]
2024-09-17 18:19:50 - sunpy - INFO: Missing metadata for solar radius: assuming the standard radius of the photosphere.
WARNING: SunpyMetadataWarning: Missing metadata for observer: assuming Earth-based observer.
For frame 'heliographic_stonyhurst' the following metadata is missing: hgln_obs,hglt_obs,dsun_obs
For frame 'heliographic_carrington' the following metadata is missing: crlt_obs,dsun_obs,crln_obs
 [sunpy.map.mapbase]
2024-09-17 18:19:50 - sunpy - WARNING: SunpyMetadataWarning: Missing metadata for observer: assuming Earth-based observer.
For frame 'heliographic_stonyhurst' the following metadata is missing: hgln_obs,hglt_obs,dsun_obs
For frame 'heliographic_carrington' the following metadata is missing: crlt_obs,dsun_obs,crln_obs

Ah I see, yeah but we should not be fixing these metadata ourselves but letting sunpy emit them. These are legit issues with the FITS files and having some special helper function for it is not something we should be doing in sunkit-magex.

nabobalis · 2024-12-30T17:10:00Z

Dob build is failing due to JSOC. I can skip the examples.

nabobalis · 2024-12-30T17:15:42Z

sunkit_magex/pfss/utils.py

@@ -36,6 +37,7 @@ def _earth_obs_coord_meta(obstime):
    return _observer_coord_meta(sunpy.coordinates.get_earth(obstime))


+@deprecated('1.0', message="This is now fixed within sunpy when you load a HMI file.")
 def fix_hmi_meta(hmi_map):


So the only fix not in sunpy core is:

# Fix observer coordinate if 'hglt_obs' not in hmi_map.meta: hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))

We can talk about adding it but that becomes an open question for the other SDO or earth based observatories.

Cadair · 2025-02-04T14:21:03Z

changelog/63.breaking.2.rst

+- pfss.utils.fix_hmi_meta
+- pfss.utils.load_adapt
+
+These functions are no longer needed and will be removed.


Are there alternatives we can direct people to?

There is an example on how to load the adapt maps. FOr the fix meta, we have the how to.

I can link to both.

Cadair · 2025-02-04T14:24:31Z

sunkit_magex/pfss/utils.py

@@ -79,6 +81,7 @@ def fix_hmi_meta(hmi_map):
        hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))


+@deprecated('1.0', message="This is not required anymore. An example of how to load an ADAPT file is available in the documentation.")


I'd either link to it or say use sunpy.map.Map or similar.

Cadair · 2025-02-04T14:27:34Z

I'm generally happy with this and we should ship a release, so if we can get the CI fixed and one of @jgieseler or @STBadman are ok with this we should merge it.

STBadman · 2025-02-04T15:02:16Z

Sorry, this has been on my to-do list to review for ages - just reading the conversation I think I agree with the changes but I'd like to try out a workflow with ADAPT and HMI and make sure I understand it all. I'm at a workshop first half of this week but I will have time on Thursday if you're ok to wait till then.

…

On Tue, Feb 4, 2025, 09:27 Stuart Mumford ***@***.***> wrote: I'm generally happy with this and we should ship a release, so if we can get the CI fixed and one of @jgieseler <https://github.com/jgieseler> or @STBadman <https://github.com/STBadman> are ok with this we should merge it. — Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANC6CA4U3A5ARJCLXRLS7A32ODE63AVCNFSM6AAAAABLLLNBVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZUGEZDQNJZGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Cadair · 2025-02-04T15:39:49Z

yep, that's fine no rush!

nabobalis · 2025-02-05T00:20:50Z

pyproject.toml

+  "scipy>=1.10.1",
+  "streamtracer>=2.2.0",
+  "sunpy[map]>=6.0.1,!=6.0.0",
+  "matplotlib>=3.5.0",  # Needed to solve the correct env for oldestdeps


I didn't know how else to handle it

nabobalis · 2025-02-05T00:22:34Z

sunkit_magex/pfss/analytic.py

+    if hasattr(scipy.special, "sph_harm_y"):
+        return -scipy.special.sph_harm_y(l, m, phi, theta)
+    if hasattr(scipy.special, "sph_harm"):
+        return -scipy.special.sph_harm(m, l, phi, theta)


nabobalis · 2025-02-05T00:23:06Z

sunkit_magex/pfss/tests/test_analytic.py

    m = 0
    c = zss**(-l-2) * ((2*l + 1) / (l + 1 + l * zss**(-2*l - 1)))
    f = analytic.Br(l, m, zss)
    phi = 0 * u.deg
    theta = 0 * u.deg
-    assert f(zss, theta, phi) == -0.5 * np.sqrt(3 / np.pi) * c
+    np.testing.assert_allclose(f(zss, theta, phi), -0.5 * np.sqrt(3 / np.pi) * c)


There is a floating point difference now at the 8th decime.

nabobalis force-pushed the sunpy_version branch 3 times, most recently from 3b12579 to d4d8d35 Compare July 24, 2024 00:09

nabobalis marked this pull request as draft July 24, 2024 00:09

nabobalis marked this pull request as ready for review July 24, 2024 00:10

nabobalis marked this pull request as draft July 24, 2024 00:10

nabobalis force-pushed the sunpy_version branch 4 times, most recently from 1ba30c9 to d28706f Compare July 24, 2024 01:17

nabobalis commented Jul 24, 2024

View reviewed changes

sunkit_magex/pfss/tests/test_pfss.py Show resolved Hide resolved

nabobalis commented Jul 24, 2024

View reviewed changes

sunkit_magex/pfss/tests/test_pfss.py Show resolved Hide resolved

nabobalis commented Jul 24, 2024

View reviewed changes

nabobalis force-pushed the sunpy_version branch from d28706f to 23a1392 Compare July 24, 2024 01:53

Cadair previously requested changes Jul 24, 2024

View reviewed changes

nabobalis force-pushed the sunpy_version branch 5 times, most recently from 6af00f1 to bfacbe8 Compare July 24, 2024 22:44

nabobalis commented Jul 24, 2024

View reviewed changes

examples/using_sunkit_magex_pfss/overplotting_onto_aia.py Outdated Show resolved Hide resolved

nabobalis force-pushed the sunpy_version branch 5 times, most recently from 2d7c5db to 1f03469 Compare July 25, 2024 19:51

nabobalis commented Jul 25, 2024

View reviewed changes

nabobalis marked this pull request as ready for review July 25, 2024 19:52

nabobalis requested a review from Cadair August 22, 2024 19:54

nabobalis mentioned this pull request Nov 11, 2024

Unable to load ADAPT map with sunpy>=6.0.0 #66

Closed

nabobalis marked this pull request as draft December 30, 2024 16:52

nabobalis force-pushed the sunpy_version branch from de8000d to d57aef3 Compare December 30, 2024 17:03

nabobalis marked this pull request as ready for review December 30, 2024 17:05

nabobalis commented Dec 30, 2024

View reviewed changes

Cadair reviewed Feb 4, 2025

View reviewed changes

nabobalis added 3 commits February 4, 2025 14:06

increase min sunpy

b420bc1

Handle old functions and update examples

9514b3c

bug in sunpy 6.0.0 means we need to wait for bugfix release

03d5661

nabobalis force-pushed the sunpy_version branch 2 times, most recently from b6256f4 to 5eb8e40 Compare February 4, 2025 22:12

CR

9552abf

nabobalis force-pushed the sunpy_version branch from 5eb8e40 to 9552abf Compare February 4, 2025 22:14

nabobalis added 4 commits February 4, 2025 15:31

Fix scipy dep warning

866743a

Update URL for example

c8e712f

fix oldestdeps

a417721

Actually fix example

7a178f9

nabobalis commented Feb 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

increase min sunpy #63

increase min sunpy #63

nabobalis commented Jul 23, 2024 •

edited

Loading

nabobalis Jul 24, 2024

Cadair Jul 24, 2024

nabobalis Jul 24, 2024

Cadair left a comment

Cadair Jul 24, 2024

nabobalis Jul 25, 2024

Cadair Feb 4, 2025

nabobalis commented Jul 25, 2024

nabobalis commented Sep 17, 2024 •

edited

Loading

jgieseler commented Sep 17, 2024

nabobalis commented Sep 17, 2024

jgieseler commented Sep 17, 2024

nabobalis commented Nov 11, 2024

nabobalis commented Dec 30, 2024

nabobalis Dec 30, 2024

Cadair Feb 4, 2025

nabobalis Feb 4, 2025

Cadair Feb 4, 2025

nabobalis Feb 4, 2025

Cadair commented Feb 4, 2025

STBadman commented Feb 4, 2025 via email

Cadair commented Feb 4, 2025

nabobalis Feb 5, 2025

nabobalis Feb 5, 2025

nabobalis Feb 5, 2025

		hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))


		def load_adapt(adapt_path):

		@@ -79,6 +81,7 @@ def fix_hmi_meta(hmi_map):
		hmi_map.meta.update(_earth_obs_coord_meta(hmi_map.meta['date-obs']))


		@deprecated('1.0', message="This is not required anymore. An example of how to load an ADAPT file is available in the documentation.")

increase min sunpy #63

Are you sure you want to change the base?

increase min sunpy #63

Conversation

nabobalis commented Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadair left a 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

nabobalis commented Jul 25, 2024

nabobalis commented Sep 17, 2024 • edited Loading

jgieseler commented Sep 17, 2024

nabobalis commented Sep 17, 2024

jgieseler commented Sep 17, 2024

nabobalis commented Nov 11, 2024

nabobalis commented Dec 30, 2024

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

Cadair commented Feb 4, 2025

STBadman commented Feb 4, 2025 via email

Cadair commented Feb 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabobalis commented Jul 23, 2024 •

edited

Loading

nabobalis commented Sep 17, 2024 •

edited

Loading