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

Parse DN correctly in header helper #7730

Merged
merged 9 commits into from
Jul 18, 2024
Merged

Parse DN correctly in header helper #7730

merged 9 commits into from
Jul 18, 2024

Conversation

wtbarnes
Copy link
Member

@wtbarnes wtbarnes commented Jul 16, 2024 •
edited
Loading

After #7585, "DN" is now correctly handled by GenericMap. However, the header helper was still applying strict checking against the FITS standard when parsing the unit string, meaning one could not pass unit='DN' into the header helper. This fixes this bug by pulling the unit parsing logic out of GenericMap so it can be used in the header helper as well.

This also removes the _map_meta_keywords dictionary which as far as I can tell is unused anywhere and was deprecated in #6743

@wtbarnes wtbarnes requested a review from a team as a code owner July 16, 2024 05:00
@wtbarnes wtbarnes added Bug map Affects the map submodule backport 6.0 on-merge: backport to 6.0 labels Jul 16, 2024
@nabobalis nabobalis added the Minor Change PR only needs one approval to merge. label Jul 16, 2024
@wtbarnes
Copy link
Member Author

wtbarnes commented Jul 16, 2024 •
edited
Loading

Wait...this does not do what I want it to do...

_parse_fits_unit still returns a Unit. It needs to be a string. Maybe this is not the right approach.

@wtbarnes wtbarnes marked this pull request as draft July 16, 2024 05:10
@nabobalis
Copy link
Contributor

Wait...this does not do what I want it to do...

_parse_fits_unit still returns a Unit. It needs to be a string. Maybe this is not the right approach.

Time for a second method

@nabobalis nabobalis requested review from nabobalis and removed request for nabobalis July 16, 2024 05:14
nabobalis
nabobalis requested changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

CI is very broken

@wtbarnes
Copy link
Member Author

Oops I forgot it was used other places.

@wtbarnes wtbarnes added this to the 6.0.0 milestone Jul 17, 2024
@wtbarnes wtbarnes marked this pull request as ready for review July 18, 2024 05:26
@nabobalis
Copy link
Contributor

setuptools 71.0.1 came out 3 hours ago and broke things, we can pin out that verison.

@wtbarnes
Copy link
Member Author

Oh lovely. I was wondering what in the world was happening.

@Cadair
Copy link
Member

Cadair commented Jul 18, 2024

you broke it: https://github.com/sunpy/sunpy/actions/runs/9992411608/job/27617589005?pr=7730#step:10:757

@wtbarnes
Copy link
Member Author

you broke it: https://github.com/sunpy/sunpy/actions/runs/9992411608/job/27617589005?pr=7730#step:10:757

How is that possible

@nabobalis
Copy link
Contributor

It needs to point to the api link and not the import. Is my guess.

@nabobalis nabobalis added the Merge When CI Passes Hit that merge button when it's all green! label Jul 18, 2024
@wtbarnes wtbarnes merged commit 1f6b904 into sunpy:main Jul 18, 2024
28 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/sunpy that referenced this pull request Jul 18, 2024
@wtbarnes @meeseeksmachine
Backport PR sunpy#7730: Parse DN correctly in header helper
b1d4cb4
@meeseeksmachine meeseeksmachine mentioned this pull request Jul 18, 2024
Merged
nabobalis added a commit that referenced this pull request Jul 18, 2024
@nabobalis
Merge pull request #7738 from meeseeksmachine/auto-backport-of-pr-773…
062eb77 
…0-on-6.0

Backport PR #7730 on branch 6.0 (Parse DN correctly in header helper)
Prtm2110 added a commit to Prtm2110/sunpy that referenced this pull request Aug 4, 2024
@Prtm2110
added gallery
8aa6cc4 
solved commit issue

Changelog

label removed

figure devdeps hash

Revert "Experimental change"

This reverts commit 779e66a.

unskip more tests

Bug fix the 1.1.0 schema to allow non-VO units

Update sunpy/tests/figure_hashes_mpl_382_ft_261_astropy_600_animators_111.json

Increase the min asdf deps

Pin out naughty setuptools

Parse DN correctly in header helper (sunpy#7730)

* parse DN correctly in header helper

* changelog

* wrong changelog number

* revert parse_fits_unit move; simplify DN checking

* revert small changes

* more newlines

* Update sunpy/map/tests/test_header_helper.py

Co-authored-by: Nabil Freij <[email protected]>

* Update changelog/7730.bugfix.rst

---------

Co-authored-by: Nabil Freij <[email protected]>

replace usages of ``copy_arrays`` with ``memmap`` (sunpy#7737)

reorder and cull whatsnew

whats new review comments

Steal the example for spice.get_rotation_matrix

Update to use minimum_dependencies

Update tox.ini

bump numpy for scipy

bump packaging

Update to use extra groups and undo pin on setuptools

more packages

modify arguments based on installed version of asdf

extra fixes for oldestdeps

Update pyproject.toml

make XRTMap have default units of DN

Update changelog/7744.bugfix.rst

Co-authored-by: Nabil Freij <[email protected]>

zenodo

Fix my affiliation

more mailmap

update json

update release notes

mailmap

Add Zach

Add Manit Singh info

Fix my mistakes

Update release numbers

Keith

Update last two

Release v6.0.0

minor suggestions

minor changes again

first animation

added ref

docfail-test

Automatic package template update

docfailing

docfail

minor changes and animation

minor changes

Add 6.0 and remove 5.X cron jobs

plt.show added

minor changes and changelog ref

changelog ref

changelog ref

contourpy is added

merge issue

added unit nitpick

fix for drms refactor

add skip windows for broken test

changelog

minor changes

ski ref updated

matplotlib mkn v and conf.py

tests

back to mpl3.5 and contourpy added to toml

tests new

added removed part

mpl to 3.6 and changelog

contour 1.0,1

swaped the axes of contourpy and changelog
Prtm2110 added a commit to Prtm2110/sunpy that referenced this pull request Aug 4, 2024
@Prtm2110
# This is a combination of 70 commits.
0e345f1 
# This is the 1st commit message:

added gallery

# This is the commit message sunpy#2:

solved commit issue

# This is the commit message sunpy#3:

Changelog

# This is the commit message sunpy#4:

label removed

# This is the commit message sunpy#5:

figure devdeps hash

# This is the commit message sunpy#6:

Revert "Experimental change"

This reverts commit 779e66a.

# This is the commit message sunpy#7:

unskip more tests

# This is the commit message sunpy#8:

Bug fix the 1.1.0 schema to allow non-VO units

# This is the commit message sunpy#9:

Update sunpy/tests/figure_hashes_mpl_382_ft_261_astropy_600_animators_111.json
# This is the commit message sunpy#10:

Increase the min asdf deps

# This is the commit message sunpy#11:

Pin out naughty setuptools

# This is the commit message sunpy#12:

Parse DN correctly in header helper (sunpy#7730)

* parse DN correctly in header helper

* changelog

* wrong changelog number

* revert parse_fits_unit move; simplify DN checking

* revert small changes

* more newlines

* Update sunpy/map/tests/test_header_helper.py

Co-authored-by: Nabil Freij <[email protected]>

* Update changelog/7730.bugfix.rst

---------

Co-authored-by: Nabil Freij <[email protected]>
# This is the commit message sunpy#13:

replace usages of ``copy_arrays`` with ``memmap`` (sunpy#7737)


# This is the commit message sunpy#14:

reorder and cull whatsnew

# This is the commit message sunpy#15:

whats new review comments

# This is the commit message sunpy#16:

Steal the example for spice.get_rotation_matrix

# This is the commit message sunpy#17:

Update to use minimum_dependencies

# This is the commit message sunpy#18:

Update tox.ini
# This is the commit message sunpy#19:

bump numpy for scipy

# This is the commit message sunpy#20:

bump packaging

# This is the commit message sunpy#21:

Update to use extra groups and undo pin on setuptools

# This is the commit message sunpy#22:

more packages

# This is the commit message sunpy#23:

modify arguments based on installed version of asdf

# This is the commit message sunpy#24:

extra fixes for oldestdeps

# This is the commit message sunpy#25:

Update pyproject.toml
# This is the commit message sunpy#26:

make XRTMap have default units of DN

# This is the commit message sunpy#27:

Update changelog/7744.bugfix.rst

Co-authored-by: Nabil Freij <[email protected]>
# This is the commit message sunpy#28:

zenodo

# This is the commit message sunpy#29:

Fix my affiliation
# This is the commit message sunpy#30:

more mailmap

# This is the commit message sunpy#31:

update json

# This is the commit message sunpy#32:

update release notes

# This is the commit message sunpy#33:

mailmap

# This is the commit message sunpy#34:

Add Zach

# This is the commit message sunpy#35:

Add Manit Singh info

# This is the commit message sunpy#36:

Fix my mistakes

# This is the commit message sunpy#37:

Update release numbers

# This is the commit message sunpy#38:

Keith

# This is the commit message sunpy#39:

Update last two

# This is the commit message sunpy#40:

Release v6.0.0

# This is the commit message sunpy#41:

minor suggestions

# This is the commit message sunpy#42:

minor changes again

# This is the commit message sunpy#43:

first animation

# This is the commit message sunpy#44:

added ref

# This is the commit message sunpy#45:

docfail-test

# This is the commit message sunpy#46:

Automatic package template update

# This is the commit message sunpy#47:

docfailing

# This is the commit message sunpy#48:

docfail

# This is the commit message sunpy#49:

minor changes and animation

# This is the commit message sunpy#50:

minor changes

# This is the commit message sunpy#51:

Add 6.0 and remove 5.X cron jobs
# This is the commit message sunpy#52:

plt.show added

# This is the commit message sunpy#53:

minor changes and changelog ref

# This is the commit message sunpy#54:

changelog ref

# This is the commit message sunpy#55:

changelog ref

# This is the commit message sunpy#56:

contourpy is added

# This is the commit message sunpy#57:

merge issue

# This is the commit message sunpy#58:

added unit nitpick
# This is the commit message sunpy#59:

fix for drms refactor

# This is the commit message sunpy#60:

add skip windows for broken test

# This is the commit message sunpy#61:

changelog

# This is the commit message sunpy#62:

minor changes

# This is the commit message sunpy#63:

ski ref updated

# This is the commit message sunpy#64:

matplotlib mkn v and conf.py

# This is the commit message sunpy#65:

tests

# This is the commit message sunpy#66:

back to mpl3.5 and contourpy added to toml

# This is the commit message sunpy#67:

tests new

# This is the commit message sunpy#68:

added removed part

# This is the commit message sunpy#69:

mpl to 3.6 and changelog

# This is the commit message sunpy#70:

contour 1.0,1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabobalis nabobalis nabobalis approved these changes

Assignees
No one assigned
Labels
backport 6.0 on-merge: backport to 6.0 map Affects the map submodule Merge When CI Passes Hit that merge button when it's all green! Minor Change PR only needs one approval to merge.
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wtbarnes @nabobalis @Cadair