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 astropy version #418

Merged
merged 3 commits into from
May 22, 2024
Merged

Update astropy version #418

merged 3 commits into from
May 22, 2024

Conversation

hugobuddel
Copy link
Collaborator

@teutoburg teutoburg added the dependencies Related to or updating any dependencies label May 22, 2024
@teutoburg teutoburg self-assigned this May 22, 2024
Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.83%. Comparing base (46561c5) to head (56ec0b1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #418   +/-   ##
=======================================
  Coverage   74.83%   74.83%           
=======================================
  Files          56       56           
  Lines        7824     7824           
=======================================
  Hits         5855     5855           
  Misses       1969     1969           

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

@hugobuddel
Copy link
Collaborator Author

What did you do to update the lock file? We should document that somewhere

@teutoburg
Copy link
Contributor

  1. Change the astropy version number to "5.3.4"
  2. Run poetry lock --no-update. This will resolve all the dependency version constraints against each other and sync the lock file with the pyproject.toml, without changing any other version that were defined with "^x.y", so no need to change those. But the one you're interested in actually changing must not have the "^" at that point, otherwise it'll go to the latest version that matches.
  3. Change the astropy version number to "^5.3.4"
  4. Run poetry lock --no-update again. This will simply sync the version constraint from pyproject.toml to the lock file (i.e. add the "^" to the lock file). You shouldn't see any other change at this point. This last step is necessary so poetry won't complain that they're out of sync (optionally run poetry check afterwards to confirm this).

I thought I'd written that down somewhere before, but couldn't find it. Maybe I'll add it here.

@teutoburg
Copy link
Contributor

Running the poetry commands mentioned above gave me the following: Warning: The locked version 2.32.0 for requests is a yanked version. [...]

This is a result of an over-enthusiastic dependabot in #417. The same PR is now also in some of our other projects. Please don't merge those, or they'll result in the same issue. I'll resolve them.

To fix the warning here, I simply ran poetry update requests. This is somewhat simpler because requests is only an indirect dependency in ScopeSim. Though the command also bumped idna, pillow and tqdm, which is fine and matches some dependabot PRs we recently got in e.g. AstarVienna/HowManyBloodyPhotons#7.

@teutoburg teutoburg merged commit 68832ab into main May 22, 2024
25 checks passed
@teutoburg teutoburg deleted the hb/updateastropy branch May 22, 2024 12:12
@hugobuddel
Copy link
Collaborator Author

I've created AstarVienna/astarvienna.github.io#4 based on your instructions in #418 (comment) @teutoburg, thanks.

About this specific issue: the ^ is preventing users from using astropy 6 right? Should we update that? Can we do something that allows both^5.3.4 and ^6.x.y (for whatever minimal 6.x.y ScopeSim works)? Because going directly to ^6 might cause troubles for users who have other packages installed as well.

@teutoburg
Copy link
Contributor

About this specific issue: the ^ is preventing users from using astropy 6 right? Should we update that? Can we do something that allows both^5.3.4 and ^6.x.y (for whatever minimal 6.x.y ScopeSim works)? Because going directly to ^6 might cause troubles for users who have other packages installed as well.

Are we sure nothing breaks with Astropy 6.x? If yes, we could change it to ">= 5.3.4, < 7.0".

@hugobuddel
Copy link
Collaborator Author

Are we sure nothing breaks with Astropy 6.x? If yes, we could change it to ">= 5.3.4, < 7.0".

We tested the latest dependencies before switching to poetry and using the caret notation, exactly so we would know early through the CI whether something would break.

To me it seems worthwhile to add such a test back, because I want to prevent learning about things breaking when we don't have time for it.

E.g. say that there would not have been an astropy 5.3.4, and we would have needed to switch to 6.x.y. If ScopeSim would not work with 6.x.y, then we would suddenly have no working version of ScopeSim anymore.

@hugobuddel
Copy link
Collaborator Author

Hmm I tried a simple

sed -i 's/ = "^/ = ">=/g' pyproject.toml 

then manually set the Python version back to ^3.9, and then ren

poetry update

But that still put only astropy 5.3.4 as a dependency. Which makes sense, because we specify ^5.3.3 in the other projects.

So we can only do such a test properly by doing such a sed replace in all our projects...

@hugobuddel
Copy link
Collaborator Author

I've created AstarVienna/ScopeSim_Data#16 as an attempt to create a test that uses the actual latest dependencies. We can discuss the dependencies there further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Related to or updating any dependencies
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants