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.

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

Add an HMISynopticMap documentation note that we are assuming that CDELT1 is the wrong sign #7471

Open
ayshih opened this issue Mar 4, 2024 · 1 comment
Labels
Documentation Affects the documentation Effort Low Requires a small time investment map Affects the map submodule Package Intermediate Requires some knowledge of the internal structure of SunPy Priority Low Slow action required

Comments

@ayshih
Copy link
Member

ayshih commented Mar 4, 2024

We currently strip the sign from CDELT1 for HMI synoptic maps because they appear to have the incorrect sign. See #4053 (comment) for relevant discussion. However, we neglected in that PR to add a note to the docstring. Let's add a note.


Original issue text:

Per discussion on Element, the metadata for HMI synoptic maps is inconsistent with how they are (officially?) plotted:

  • The metadata has negative CDELT1, which would means that the longitude axis should be plotted decreasing from 360 deg to 0 deg from left to right
  • The JSOC-hosted PNGs of the same files show the longitude axis increasing from 0 deg to 360 deg from left to right (e.g., http://jsoc.stanford.edu/data/hmi/synoptic/hmi.Synoptic_Mr.2191.png)

Our code currently (unintentionally?) strips the sign from CDELT1 by calling np.abs() here:

return SpatialPair(np.abs(self.meta['cdelt1']) * self.spatial_units[0] / u.pixel,
180 / np.pi * self.meta['cdelt2'] * u.deg / u.pixel)

That is, our output matches the JSOC-hosted PNGs, but if this is the "correct" way to plot synoptic maps, we should add a note to the docstring saying that we are intentionally not following CDELT1 as specified in the metadata.

@ayshih ayshih added the map Affects the map submodule label Mar 4, 2024
@ayshih
Copy link
Member Author

ayshih commented Mar 4, 2024

Thanks, @alasdairwilson, for reminding me that I already investigated this (#4053 (comment)), and I concluded that the CDELT1 in the metadata is actually wrong (i.e., provides the incorrect mapping between data and coordinates). This is not merely an issue of a plotting convention.

@ayshih ayshih changed the title Add an HMISynopticMap documentation note about the longitude direction Add an HMISynopticMap documentation note that we are assuming that CDELT1 is the wrong sign Mar 4, 2024
@nabobalis nabobalis added Documentation Affects the documentation Priority Low Slow action required Effort Low Requires a small time investment Package Intermediate Requires some knowledge of the internal structure of SunPy labels Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Affects the documentation Effort Low Requires a small time investment map Affects the map submodule Package Intermediate Requires some knowledge of the internal structure of SunPy Priority Low Slow action required
Projects
None yet
Development

No branches or pull requests

2 participants