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

replace usages of copy_arrays with memmap #360

Merged

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Jul 18, 2024

follow-on to asdf-format/asdf#1797

Checklist

@zacharyburnett zacharyburnett self-assigned this Jul 18, 2024
Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.20%. Comparing base (087a60d) to head (7608ae5).
Report is 58 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
- Coverage   97.56%   97.20%   -0.36%     
==========================================
  Files          30       37       +7     
  Lines        2788     3359     +571     
==========================================
+ Hits         2720     3265     +545     
- Misses         68       94      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@braingram
Copy link
Collaborator

Bumping the asdf version to at least 3.1.0 should fix the oldest deps:

"asdf >=2.15.0",

@zacharyburnett zacharyburnett marked this pull request as ready for review July 18, 2024 19:54
@zacharyburnett zacharyburnett requested a review from a team as a code owner July 18, 2024 19:54
@PaulHuwe
Copy link
Collaborator

Can you please add a RCAL regression test URL as well?

@zacharyburnett
Copy link
Collaborator Author

Can you please add a RCAL regression test URL as well?

RCAL regtest running at https://github.com/spacetelescope/RegressionTests/actions/runs/10042547798 with this branch installed

@zacharyburnett
Copy link
Collaborator Author

@zacharyburnett zacharyburnett force-pushed the deprecate/copy_arrays branch from 0a25d81 to 086ab4b Compare July 22, 2024 17:17
@braingram
Copy link
Collaborator

a few failures in RCAL regtests: https://github.com/spacetelescope/RegressionTests/actions/runs/10042547798/attempts/2#summary-27759909972

The failures are unrelated (they're expected and caused by spacetelescope/romancal#1241).

Would you re-run them? I okified the results from a different run but I think that wasn't until after your tests had run.

@zacharyburnett
Copy link
Collaborator Author

@braingram
Copy link
Collaborator

The oldest deps failure should be solvable with a bump to the gwcs version:
https://github.com/spacetelescope/roman_datamodels/actions/runs/10045391464/job/27762571944?pr=360#step:10:292

@braingram
Copy link
Collaborator

Sorry for the back and forth but this will likely need another run. It turns out that some of the roman tests continue to fail after the first "okify": spacetelescope/romancal#1325 I have a run on main to test if the current "okified" results pass. I'll post and update here once the run on main passes.

@braingram
Copy link
Collaborator

I re-ran the regtests for this PR (now that the romancal main ones are passing):
https://github.com/spacetelescope/RegressionTests/actions/runs/10042547798

The run with this PR shows all passing tests.

Copy link
Collaborator

@braingram braingram left a comment

Choose a reason for hiding this comment

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

LGTM!

@zacharyburnett zacharyburnett merged commit 96026c9 into spacetelescope:main Jul 23, 2024
15 of 16 checks passed
@zacharyburnett zacharyburnett deleted the deprecate/copy_arrays branch July 23, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants