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

Update for webbpsf 1.2.1. #71

Merged
merged 7 commits into from
Aug 21, 2023
Merged

Conversation

schlafly
Copy link
Collaborator

This updates the webbpsf data files used in CI to the latest versions.

@schlafly
Copy link
Collaborator Author

@zacharyburnett , would you mind taking a look at this? webbpsf 1.2.1 came out and needed a corresponding bump to the data file that the CI downloads. I tried to make that change there but somehow the CI is still seeing 1.1.1 (error message from CI: OSError: WebbPSF data package has version 1.1.1, but 1.2.1 is needed). I can look at the CI logs and see that it is downloading the new files in the CI for this PR, and when I download those files myself they work fine with webbpsf 1.2.1. Do I need to do something special to "clear the cache" or something to get it to see the right files, or have I screwed something else up? Thanks!

@schlafly
Copy link
Collaborator Author

After clearing the cache I got this to work better. I suspect the hashing going on in the CI is not working quite right and it should have been recognizing that the new files were different but it wasn't actually doing that. There is still some additional issue where the tox codecov runs are failing, but I don't understand that. @zacharyburnett , sorry, I can't make heads or tails of this one.

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3471bc7) 90.37% compared to head (5583461) 90.37%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #71   +/-   ##
=======================================
  Coverage   90.37%   90.37%           
=======================================
  Files          16       16           
  Lines        1444     1444           
=======================================
  Hits         1305     1305           
  Misses        139      139           

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

@schlafly
Copy link
Collaborator Author

schlafly commented Aug 17, 2023

Okay, this is now solved. At least four relevant issues:

  • The WebbPSF link needed to be updated due to their 1.2.1 release. It seems to me that they could have a -latest-released link that could reduce some churn there?
  • The hashing used to make the cache doesn't look quite right and didn't recognize that the new file downloaded as part of 1.2.1 wasn't identical to the old one (?). The tox jobs then got the old data and were still out of date after updating the links and downloading the now files. Clearing the cache in the actions tab caused this to grab a new version and succeed.
  • sphinx had an update that was breaking the theme used for the readthedocs build. I pinned to versions less than 7.
  • tox had an update that identified an issue where we weren't appropriately listing all of the tox environments correctly. This PR tweaks the names of the tox environments called by the ci to be consistent, and adds an environment to the tox.ini file.

I think that's everything! But I don't really know what I'm doing with any of these.

@schlafly schlafly marked this pull request as ready for review August 17, 2023 15:17
@schlafly schlafly requested a review from a team as a code owner August 17, 2023 15:17
Copy link
Collaborator

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

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

LGTM - I defer to @zacharyburnett on the CI issues.

@schlafly schlafly merged commit 8fc4f94 into spacetelescope:main Aug 21, 2023
@schlafly schlafly deleted the update-webbpsf branch August 21, 2023 13:34
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.

2 participants