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

replace usages of copy_arrays with memmap #7737

Merged
merged 1 commit into from
Jul 18, 2024
Merged

replace usages of copy_arrays with memmap #7737

merged 1 commit into from
Jul 18, 2024

Conversation

zacharyburnett
Copy link
Contributor

PR Description

follow-on to asdf-format/asdf#1797

@zacharyburnett zacharyburnett force-pushed the deprecate/copy_arrays branch from 2d9719e to 18f09da Compare July 18, 2024 15:22
@nabobalis nabobalis added Minor Change PR only needs one approval to merge. backport 6.0 on-merge: backport to 6.0 io Affects the io submodule labels Jul 18, 2024
@nabobalis nabobalis marked this pull request as ready for review July 18, 2024 15:23
@nabobalis nabobalis requested a review from a team as a code owner July 18, 2024 15:23
@nabobalis nabobalis added Tests Affects tests in some measure. No Changelog Entry Needed Skip any changelog checks. Merge When CI Passes Hit that merge button when it's all green! labels Jul 18, 2024
@zacharyburnett zacharyburnett force-pushed the deprecate/copy_arrays branch from 18f09da to 890ddaf Compare July 18, 2024 15:26
@nabobalis
Copy link
Contributor

Thanks for the PR @zacharyburnett

@wtbarnes wtbarnes merged commit e587858 into sunpy:main Jul 18, 2024
43 of 46 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/sunpy that referenced this pull request Jul 18, 2024
@zacharyburnett @meeseeksmachine
Backport PR sunpy#7737: replace usages of copy_arrays with ``memm…
699a409 
…ap``
@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 #7739 from meeseeksmachine/auto-backport-of-pr-773…
70c8a7e 
…7-on-6.0

Backport PR #7737 on branch 6.0 (replace usages of ``copy_arrays`` with ``memmap``)
@zacharyburnett zacharyburnett deleted the deprecate/copy_arrays branch July 18, 2024 18:16
@zacharyburnett
Copy link
Contributor Author

zacharyburnett commented Jul 18, 2024 •
edited
Loading

@nabobalis I apologize, I forgot to include that memmap only exists in asdf>=3.1.0; there shoul probably be a pin in pyproject.toml to that effect, or alternatively I can add a condition to check the asdf version and use copy_arrays or memmap accordingly

@nabobalis
Copy link
Contributor

As long as our test suite passed, which I think it did even on our oldest deps, I don't see the issue?

@nabobalis
Copy link
Contributor

nabobalis commented Jul 18, 2024 •
edited
Loading

The oldest deps despite the pin, used asdf==3.3.0, I assume maybe python 310 cant install older asdf versions?

@zacharyburnett
Copy link
Contributor Author

it looks like oldestdeps in tox.ini pins asdf<2.12.0

sunpy/tox.ini

Line 61 in e587858

oldestdeps: asdf<2.12.0

however the project requires asdf>=2.13.0

sunpy/pyproject.toml

Line 58 in e587858

"asdf>=2.13.0",

and since the pip install . step occurs after the dependencies from toxenv:deps are installed, this means the oldestdeps in tox.ini are overwritten:
https://github.com/sunpy/sunpy/actions/runs/9993966424/job/27622904664#step:10:21

@nabobalis
Copy link
Contributor

Wait, our CI wasn't updated to use the new min versions?!

I will have to change that.

Either way, this is just test changes so I have no issue as long as our tests pass.

@zacharyburnett
Copy link
Contributor Author

We could use something like https://github.com/OpenAstronomy/minimum_dependencies to gather the oldest deps and install those (see https://github.com/spacetelescope/jwst/blob/de39de07b6f0380860caa86e8485666b4cef35c2/tox.ini#L51-L64)

@nabobalis
Copy link
Contributor

We already do elsewhere, it just wasn't added to sunpy for some reason I don't know.

@zacharyburnett zacharyburnett mentioned this pull request Jul 18, 2024
Closed
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 #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 io Affects the io submodule Merge When CI Passes Hit that merge button when it's all green! Minor Change PR only needs one approval to merge. No Changelog Entry Needed Skip any changelog checks. Tests Affects tests in some measure.
Projects
None yet
Milestone
No milestone
Development

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