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

Fix for checking inputs to header_helper, removing ability to add all kwargs and only ones defined #3183

Merged
merged 3 commits into from Jun 7, 2019

Conversation

Projects
None yet
4 participants
@hayesla
Copy link
Contributor

commented Jun 3, 2019

Description

The header_helper.make_fitswcs_header function was able to take any kwarg that was in the list of accepted meta data and then populated the MetaDict at the end of the function with any of these keywords, and hence allowed the user to overwrite the meta pulled from the SkyCoord, and have conflicting meta keywords. it was as such

    for key in kwargs:
        if key in _map_meta_keywords:
            meta_dict[key] = kwargs[key]

This is now removed in this PR.

I've also added that the keywords it will take are listed in the function, with both exposure and wavelength being astropy quantities.

Fixes #3168

@sunpy-bot

This comment has been minimized.

Copy link

commented Jun 3, 2019

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

@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

ping @ehsteve

@nabobalis nabobalis added this to the 1.0.1 milestone Jun 3, 2019

@nabobalis nabobalis requested review from ehsteve and Cadair Jun 3, 2019

Show resolved Hide resolved sunpy/map/header_helper.py Outdated
Show resolved Hide resolved sunpy/map/header_helper.py Outdated
@pep8speaks

This comment has been minimized.

Copy link

commented Jun 3, 2019

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

Line 37:101: E501 line too long (106 > 100 characters)

Line 39:47: E252 missing whitespace around parameter equals
Line 39:48: E252 missing whitespace around parameter equals
Line 39:67: E252 missing whitespace around parameter equals
Line 39:68: E252 missing whitespace around parameter equals

Comment last updated at 2019-06-07 09:28:52 UTC
@Cadair

Cadair approved these changes Jun 6, 2019

@Cadair

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

There is an example which is failing to build here.

@nabobalis nabobalis force-pushed the hayesla:fix_inputs_header_helper branch from 7ee207c to 40d644e Jun 7, 2019

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

Rebased and fixed up the failing example.

@nabobalis nabobalis force-pushed the hayesla:fix_inputs_header_helper branch from 40d644e to 056c91c Jun 7, 2019

@nabobalis nabobalis force-pushed the hayesla:fix_inputs_header_helper branch from 056c91c to 0ef452a Jun 7, 2019

@nabobalis nabobalis merged commit dc0d5b5 into sunpy:master Jun 7, 2019

16 checks passed

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/patch 85.71% of diff hit (target 79.83%)
Details
codecov/project 90.35% (+10.52%) compared to d8d5fd2
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20190607.15 succeeded
Details
sunpy.sunpy (Linux_36_Conda_offline) Linux_36_Conda_offline succeeded
Details
sunpy.sunpy (Linux_36_online) Linux_36_online succeeded
Details
sunpy.sunpy (Linux_37_offline) Linux_37_offline succeeded
Details
sunpy.sunpy (Windows_36_offline) Windows_36_offline succeeded
Details
sunpy.sunpy (macOS_37_offline) macOS_37_offline succeeded
Details
@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2019

Thanks @hayesla

nabobalis added a commit to nabobalis/sunpy that referenced this pull request Jun 7, 2019

Merge pull request sunpy#3183 from hayesla/fix_inputs_header_helper
Fix for checking inputs to header_helper, removing ability to add all kwargs and only ones defined

nabobalis added a commit to nabobalis/sunpy that referenced this pull request Jun 7, 2019

Merge pull request sunpy#3183 from hayesla/fix_inputs_header_helper
Fix for checking inputs to header_helper, removing ability to add all kwargs and only ones defined
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.