No more modify meta #5501

dstansby · 2021-08-12T19:25:15Z

Replaces #5199 - see that PR for motivation and discussion.

Fixes #4324, fixes #4410

pep8speaks · 2021-08-12T19:25:20Z

Hello @dstansby! Thanks for updating this PR.

In the file sunpy/map/sources/hinode.py:

Line 10:17: E741 ambiguous variable name 'l'

Comment last updated at 2021-10-19 17:52:17 UTC

wtbarnes · 2021-08-19T16:59:36Z

There are also still some modification to header values in io, e.g. the fixing of the WAVEUNIT keyword:

sunpy/sunpy/io/fits.py

Line 143 in a319336

header['WAVEUNIT'] = waveunit

I don't know if you want to address that here as well?

dstansby · 2021-08-19T17:27:40Z

Thanks - I hadn't spotted that, but I will try to remove that in this PR too.

wtbarnes · 2021-08-19T18:00:14Z

It came about when I was constructing the EUI headers (which are for some reason missing the WAVEUNIT key). When you pass a file into the map factory, it passes through io and gets the wave unit fix. But if you just pass the data header pair, this fix is not applied and things blow up when you try to access the wavelength property.

dstansby · 2021-08-19T18:18:54Z

I had a quick ctrl-F and WAVEUNIT doesn't seem to be in the FITS spec anywhere, so I guess it's just a commonly used but not official keyword?

wtbarnes · 2021-08-20T20:00:54Z

Oh there are going to be some conflicts between this and #5261 😅

docs/whatsnew/3.1.rst

nabobalis · 2021-10-16T04:44:02Z

sunpy/map/sources/iris.py

-from sunpy.map import GenericMap
+import astropy.units as u
+
+from sunpy.map.mapbase import GenericMap, SpatialPair

 __all__ = ['SJIMap']


Should we deprecate this?

Why? Either way I think that's something for a new PR.

nabobalis · 2021-10-16T04:45:01Z

sunpy/map/sources/mlso.py

+        return u.Unit(unit)
+
+    @property
+    def _default_observer_coordinate(self):


Is this going to be useful in future or is it called elsewhere? I tried to do a search on github but it came up with nothing.

It's used in GenericMap.observer_coordinate.

nabobalis · 2021-10-16T04:46:46Z

sunpy/map/sources/soho.py

+
+    @property
+    def scale(self):
+        if self.meta['cunit2'] == 'Sine Latitude':


Should we return none if this isn't the case?

No, a user could have already corrected the metadata in which case we want to let it pass through.

wtbarnes

Some minor nitpicks, but overall looks good to me. Thanks for this massive effort!

sunpy/map/mapbase.py

sunpy/map/sources/sdo.py

Co-authored-by: Will Barnes <will.t.barnes@gmail.com>

nabobalis · 2021-10-19T16:44:22Z

FAILED ../../.tox/py39/lib/python3.9/site-packages/sunpy/map/sources/tests/test_eui_source.py::test_unit
FAILED ../../.tox/py39/lib/python3.9/site-packages/sunpy/map/sources/tests/test_wispr_source.py::test_unit
FAILED ../../.tox/py39/lib/python3.9/site-packages/sunpy/io/special/asdf/tags/tests/test_genericmap.py::test_genericmap_basic
FAILED ../../.tox/py39/lib/python3.9/site-packages/sunpy/io/special/asdf/tags/tests/test_genericmap.py::test_genericmap_mask

CI failures to fix before we merge.

dstansby requested a review from a team as a code owner August 12, 2021 19:25

dstansby mentioned this pull request Aug 12, 2021

Don't modify the header in the constructor of GenericMap subclasses #5199

Closed

dstansby marked this pull request as draft August 12, 2021 20:10

dstansby force-pushed the no-more-modify-meta branch 3 times, most recently from 410cadb to 1446531 Compare August 17, 2021 12:36

dstansby 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 Aug 17, 2021

dstansby force-pushed the no-more-modify-meta branch 2 times, most recently from 79b4d5b to 86029ba Compare August 18, 2021 09:21

dstansby mentioned this pull request Aug 18, 2021

Add some WCS smoke tests for map sources #5506

Merged

dstansby force-pushed the no-more-modify-meta branch from 86029ba to 165cbee Compare August 19, 2021 13:46

dstansby force-pushed the no-more-modify-meta branch from 165cbee to 29e1155 Compare August 19, 2021 17:59

dstansby force-pushed the no-more-modify-meta branch 3 times, most recently from 3b11bd1 to 1c03a27 Compare August 19, 2021 20:36

dstansby force-pushed the no-more-modify-meta branch 2 times, most recently from 455076b to 8f176fd Compare August 21, 2021 21:24

dstansby mentioned this pull request Aug 25, 2021

Add SEP for metadata handling sunpy/sunpy-SEP#50

Closed

dstansby mentioned this pull request Sep 6, 2021

Adding support for GOES SXI Maps #2503

Closed

5 tasks

dstansby force-pushed the no-more-modify-meta branch 2 times, most recently from f3f498a to 0a32316 Compare October 1, 2021 08:37

dstansby added this to the 3.1 milestone Oct 1, 2021

dstansby marked this pull request as ready for review October 1, 2021 15:43

dstansby requested a review from a team as a code owner October 1, 2021 15:43

dstansby added 3 commits October 14, 2021 10:48

Don't fall back on CROTA2 if CROTA keyword specified explicitly

20dc7d0

Add changelog about automatically setting the WCS

a9cd8a5

Improve whatsnew

afbfbae

Cadair reviewed Oct 14, 2021

View reviewed changes

docs/whatsnew/3.1.rst Outdated Show resolved Hide resolved

Fix line wrapping

8586a83

dstansby force-pushed the no-more-modify-meta branch from 18944ee to 8586a83 Compare October 14, 2021 13:49

Cadair approved these changes Oct 14, 2021

View reviewed changes

Fix doc link

af56754

nabobalis reviewed Oct 16, 2021

View reviewed changes

Cadair requested a review from nabobalis October 19, 2021 08:58

wtbarnes reviewed Oct 19, 2021

View reviewed changes

sunpy/map/mapbase.py Outdated Show resolved Hide resolved

sunpy/map/mapbase.py Outdated Show resolved Hide resolved

sunpy/map/mapbase.py Show resolved Hide resolved

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

nabobalis removed request for a team and nabobalis October 19, 2021 15:51

dstansby and others added 2 commits October 19, 2021 17:21

Make unit fixing case insensitive

2682f1b

Co-authored-by: Will Barnes <will.t.barnes@gmail.com>

Add clarifying comment to date_obs

a6f5946

dstansby mentioned this pull request Oct 19, 2021

Error gracefully if a map doesn't have spatial axes #5602

Merged

nabobalis approved these changes Oct 19, 2021

View reviewed changes

dstansby force-pushed the no-more-modify-meta branch from 1de6635 to a6f5946 Compare October 19, 2021 17:47

Remove redundant AIA unit code

cdf712f

nabobalis merged commit b0dbce4 into sunpy:main Oct 19, 2021

dstansby deleted the no-more-modify-meta branch October 19, 2021 19:48

ayshih mentioned this pull request Oct 25, 2021

reproject_to does not preserve metadata #5627

Open

dstansby removed the Needs Review Needs reviews before merge label Jan 24, 2022

ayshih mentioned this pull request Oct 6, 2022

Fix the K-Cor Map source to default to MLSO geographic location #6447

Merged

ayshih mentioned this pull request Oct 27, 2022

Optionally preserve metadata in reproject_to #6502

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No more modify meta #5501

No more modify meta #5501

dstansby commented Aug 12, 2021 •

edited

Loading

pep8speaks commented Aug 12, 2021 •

edited

Loading

wtbarnes commented Aug 19, 2021

dstansby commented Aug 19, 2021

wtbarnes commented Aug 19, 2021

dstansby commented Aug 19, 2021

wtbarnes commented Aug 20, 2021

nabobalis Oct 16, 2021

dstansby Oct 16, 2021

nabobalis Oct 16, 2021 •

edited

Loading

dstansby Oct 16, 2021

nabobalis Oct 16, 2021

dstansby Oct 16, 2021

wtbarnes left a comment

nabobalis commented Oct 19, 2021

No more modify meta #5501

No more modify meta #5501

Conversation

dstansby commented Aug 12, 2021 • edited Loading

pep8speaks commented Aug 12, 2021 • edited Loading

Comment last updated at 2021-10-19 17:52:17 UTC

wtbarnes commented Aug 19, 2021

dstansby commented Aug 19, 2021

wtbarnes commented Aug 19, 2021

dstansby commented Aug 19, 2021

wtbarnes commented Aug 20, 2021

nabobalis Oct 16, 2021

Choose a reason for hiding this comment

dstansby Oct 16, 2021

Choose a reason for hiding this comment

nabobalis Oct 16, 2021 • edited Loading

Choose a reason for hiding this comment

dstansby Oct 16, 2021

Choose a reason for hiding this comment

nabobalis Oct 16, 2021

Choose a reason for hiding this comment

dstansby Oct 16, 2021

Choose a reason for hiding this comment

wtbarnes left a comment

Choose a reason for hiding this comment

nabobalis commented Oct 19, 2021

dstansby commented Aug 12, 2021 •

edited

Loading

pep8speaks commented Aug 12, 2021 •

edited

Loading

nabobalis Oct 16, 2021 •

edited

Loading