Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix the way we set date on WCS #7606

Closed
wants to merge 3 commits into from
Closed

Fix the way we set date on WCS #7606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f35885
Fix the way we set date on WCS
Cadair Apr 26, 2024
4626fd5
Add changelog
Cadair Apr 26, 2024
7d0831d
Add a test
Cadair Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/7606.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug in `~.GenericMap.wcs` where the ``.dateobs`` property of the `~astropy.wcs.WCS` object was being set from the `~.GenericMap.date` property which preferrs the ``DATE-OBS`` header key over the more correct ``DATE-AVG`` if both exist.
16 changes: 13 additions & 3 deletions sunpy/map/mapbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,13 +608,23 @@ def wcs(self):
w2.wcs.crval = u.Quantity([self._reference_longitude, self._reference_latitude])
w2.wcs.ctype = self.coordinate_system
w2.wcs.pc = self.rotation_matrix
w2.wcs.set_pv(self._pv_values)
# FITS standard doesn't allow both PC_ij *and* CROTA keywords
w2.wcs.crota = (0, 0)
w2.wcs.cunit = self.spatial_units
w2.wcs.dateobs = self.date.isot
w2.wcs.aux.rsun_ref = self.rsun_meters.to_value(u.m)
w2.wcs.set_pv(self._pv_values)

# If date average exists we should use it. If it doesn't then we
# fallback to dateobs because it can come from many places
if self.date_average is not None:
w2.wcs.dateavg = self.date_average.isot
else:
w2.wcs.dateobs = self.date.isot
Comment on lines +618 to +621
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason that we conditionally set dateobs? Can we not always set this and then let the map source/.date decide what actually gets propagated up here?

Suggested change
if self.date_average is not None:
w2.wcs.dateavg = self.date_average.isot
else:
w2.wcs.dateobs = self.date.isot
w2.wcs.dateobs = self.date.isot
if self.date_average is not None:
w2.wcs.dateavg = self.date_average.isot

Copy link
Member

@ayshih ayshih Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that @Cadair is doing this – and one reason for my discomfort with this PR – is because the SPICE map doesn't actually have a source subclass. So, he wants GenericMap to put the "correct" time(s) in the WCS despite the .date property returning the "incorrect" time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So would you rather we add a spice source map?

if self.date_start is not None:
w2.wcs.datebeg = self.date_start.isot
if self.date_end is not None:
w2.wcs.dateend = self.date_end.isot

w2.wcs.aux.rsun_ref = self.rsun_meters.to_value(u.m)
# Set observer coordinate information except when we know it is not appropriate (e.g., HGS)
sunpy_frame = sunpy.coordinates.wcs_utils._sunpy_frame_class_from_ctypes(w2.wcs.ctype)
if sunpy_frame is None or hasattr(sunpy_frame, 'observer'):
Expand Down
15 changes: 15 additions & 0 deletions sunpy/map/tests/test_mapbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,21 @@ def test_date(generic_map, keys, expected_date):
else:
assert generic_map.date == expected_date

# Validate that the WCS date properties reflect the same information as the
# map properties
if "DATE-AVG" in keys or ("DATE-BEG" in keys and "DATE-END" in keys):
assert parse_time(generic_map.wcs.wcs.dateavg) - expected_date < 1*u.s
assert not generic_map.wcs.wcs.dateobs
else:
assert parse_time(generic_map.wcs.wcs.dateobs) - expected_date < 1*u.s

if "DATE-BEG" in keys:
assert generic_map.wcs.wcs.datebeg

if "DATE-END" in keys:
assert generic_map.wcs.wcs.dateend



def test_date_scale(generic_map):
# Check that default time scale is UTC
Expand Down
Loading