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

Make AIAMap use the HAE coordinates in the header #3056

Merged
merged 10 commits into from May 17, 2019

Conversation

Projects
4 participants
@ayshih
Copy link
Contributor

commented Apr 17, 2019

Fixes #3038 by overriding the properties in AIAMap to use the HAE coordinates in the header rather than the HGS coordinates.

No action is currently taken with respect to the HGS coordinates (and HGC coordinates) in the header (HGLN_OBS et al.). Possible actions include:

  • Do nothing (i.e., retain the inaccurate HGS coordinates in the metadata despite the inconsistency)
  • Remove the HGS coordinates from the metadata to have a "clean" header
  • Modify the HGS coordinates in the metadata to be more accurate

@ayshih ayshih added the map label Apr 17, 2019

@ayshih ayshih requested a review from Cadair Apr 17, 2019

@sunpy-bot

This comment has been minimized.

Copy link

commented Apr 17, 2019

Thanks for the pull request @ayshih! Everything looks great!

@sunpy sunpy deleted a comment from sunpy-bot bot Apr 17, 2019

@nabobalis nabobalis added this to the 1.0 milestone Apr 18, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

My default position on the HGS keys is to do nothing. I am open to counter arguments though.

@ayshih

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

Need to fix this to use HeliocentricMeanEcliptic instead of HeliocentricTrueEcliptic for Astropy 3.2 (see #3066 and #3075)

@Cadair Cadair added this to High Priority Features in SunPy 1.0 May 1, 2019

@pep8speaks

This comment has been minimized.

Copy link

commented May 16, 2019

Hello @ayshih! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 46:101: E501 line too long (101 > 100 characters)
Line 51:101: E501 line too long (117 > 100 characters)
Line 54:101: E501 line too long (161 > 100 characters)
Line 59:80: E203 whitespace before ','
Line 59:101: E501 line too long (157 > 100 characters)
Line 84:101: E501 line too long (117 > 100 characters)
Line 85:101: E501 line too long (113 > 100 characters)
Line 88:101: E501 line too long (104 > 100 characters)
Line 90:101: E501 line too long (102 > 100 characters)
Line 91:101: E501 line too long (102 > 100 characters)

Comment last updated at 2019-05-16 19:43:55 UTC

@ayshih ayshih added the [Review] label May 16, 2019

@ayshih ayshih force-pushed the ayshih:aia_header branch from f325266 to 7e231ef May 16, 2019

@Cadair

Cadair approved these changes May 17, 2019

@Cadair Cadair added [BugFix] and removed [Review] labels May 17, 2019

@Cadair Cadair merged commit 6bde39b into sunpy:master May 17, 2019

15 of 16 checks passed

codecov/patch 72.22% of diff hit (target 89.92%)
Details
ci/circleci: 32bit Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-37 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
ci/circleci: pip-install Your tests passed on CircleCI!
Details
codecov/project 89.89% (-0.04%) compared to 89b2932
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20190516.18 succeeded
Details
sunpy.sunpy (Linux_36_Conda_offline) Linux_36_Conda_offline succeeded
Details
sunpy.sunpy (Linux_36_offline) Linux_36_offline succeeded
Details
sunpy.sunpy (Linux_37_online) Linux_37_online succeeded
Details
sunpy.sunpy (Windows_36_offline) Windows_36_offline succeeded
Details
sunpy.sunpy (macOS_37_offline) macOS_37_offline succeeded
Details

@Cadair Cadair moved this from Post-Feature Freeze to Finished in SunPy 1.0 May 17, 2019

@ayshih ayshih deleted the ayshih:aia_header branch May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.