dstansby / pfsspy

Potential Field Source Surface model package for Python
https://pfsspy.readthedocs.io/
Other
41 stars 17 forks source link

`carr_cea_wcs_header` function provides shape in wrong order to `sunpy.map.make_fitswcs_header` #300

Closed wtbarnes closed 3 years ago

wtbarnes commented 3 years ago

The docstring for carr_cea_wcs_header says

    shape : tuple
        Map shape. The first entry should be number of points in longitude, the
        second in latitude.

i.e. shape should be in cartesian order. This is then passed directly to make_fitswcs_header,

https://github.com/dstansby/pfsspy/blob/961f07522ede53fdb3cde567cf3297c711a687f0/pfsspy/utils.py#L118

However, make_fitswcs_header assumes the ordering of this input is array-index ordered: https://docs.sunpy.org/en/stable/api/sunpy.map.make_fitswcs_header.html#sunpy.map.make_fitswcs_header.

Additionally, the input can't just be reversed because the scale argument (correctly) assumes that the ordering is cartesian: https://github.com/dstansby/pfsspy/blob/961f07522ede53fdb3cde567cf3297c711a687f0/pfsspy/utils.py#L119-L120, consistent with make_fitswcs_header.

dstansby commented 3 years ago

Just fixing this, but out of interest, how did you find the issue? It doesn't seem to actually have any impact, because mak_fitswcs_header doesn't populate the NAXISi keywords.

wtbarnes commented 3 years ago

Ha well I actually found it just by looking at the source code and had been looking closely at make_fitswcs_header as well and noticed it was inconsistent.