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 v0.17.1 release with pinned scipy version #48

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

mattpitkin
Copy link
Contributor

@mattpitkin mattpitkin commented Apr 8, 2024

Would it be possible to update the release for v0.17.1 with scipy pinned to <1.13 to avoid arviz-devs/arviz#2336 for conda-forge installs on Python 3.9?

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

 - create new release number for 0.17.1 with scipy pinned to <1.13
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@mattpitkin
Copy link
Contributor Author

@conda-forge-admin, please rerender

Copy link
Contributor

github-actions bot commented Apr 8, 2024

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/arviz-feedstock/actions/runs/8602208407.

@ColCarroll
Copy link
Member

hey matt! this seems fine to me in principle. I'm double checking that merging this won't make future conda releases harder (i mostly view conda forge as magic...)

if you (or anyone) knows that this is fine, that'd be helpful to hear!

@mattpitkin
Copy link
Contributor Author

I think (although I can't be 100% sure) that the automatic bot that updates to recipes for new releases should be able to deal with this without any issues. Another option is just to revert this change once the updated 0.17.1 package has been successfully uploaded to conda forge.

@ColCarroll ColCarroll merged commit 753f936 into conda-forge:main Apr 8, 2024
3 checks passed
@ColCarroll
Copy link
Member

works for me! thanks for doing the legwork here.

@mattpitkin mattpitkin deleted the pinned_scipy branch April 8, 2024 15:21
@ColCarroll
Copy link
Member

@OriolAbril helpfully pointed out that https://github.com/conda-forge/conda-forge-repodata-patches-feedstock/blob/main/recipe/README.md is the "right" way to do this. He says: "AFAIK, the PR will update the metadata for 0.17.1, but 0.17.0, 0.16.0... will still have the issue, the metadata patch is general".

Anyways, I think this is fine, but if you have a reason to want this in general (or if either of us needs to do this again), now we know.

@mattpitkin
Copy link
Contributor Author

@ColCarroll Thanks, that is definitely useful to know!

Yes, my PR will only work for v0.17.1, which for the purpose I requested it, which was primarily to fix a CI test for another package, is fine. So, for now, I'm happy with how things are.

@ColmTalbot
Copy link

Hi @ColCarroll I was about to comment along the lines of what you said, it would be great if we could patch the metadata.

Some googling reveals that having a new build doesn't automatically deprecate the old build and having a newer version of scipy takes precedence over a newer build of arviz in the solver so we're still seeing the old build install.

@OriolAbril
Copy link
Contributor

opened a PR to patch: conda-forge/conda-forge-repodata-patches-feedstock#705. Note however I have never done a patch before so I did my best to follow the instructions and hope it is ok, I also have no idea how long it takes for patch PRs to be merged and therefore be in effect

@mattpitkin
Copy link
Contributor Author

@OriolAbril Great, thanks.

@maresb
Copy link
Contributor

maresb commented Jun 4, 2024

For future reference the preferred conda-forge procedure for generating a build of a non-latest version is to create a new branch from the corresponding commit, e.g. v0.17.1 or v0.17.x, and merging such a PR to that. It's nothing critical, but it makes the history easier to follow. (I was actually confused for a while and thought that the dependencies on v0.18.0 were wrong.)

@maresb
Copy link
Contributor

maresb commented Jun 4, 2024

For possible future reference, the diff of this PR relative to v0.17.1 is:

https://github.com/conda-forge/arviz-feedstock/compare/a2b3d83..753f936

the primary change being to bump the build number to 1 and do

-    - scipy >=1.8
+    - scipy >=1.9,<1.13

The repodata patch adds <1.13 to build 0.

Note: build 0 allows scipy 1.8, but the requirements of v0.17.1 specify >=1.9. Thus build 0 is broken.

@OriolAbril
Copy link
Contributor

As a bugfix, the requirements for 0.17.1 are the same as 0.17.0, thus the requirements are >=1.8, ref https://github.com/arviz-devs/arviz/blob/v0.17.1/requirements.txt#L4. It is the build 1 that is inconsistent with pypi now. Plus having added the repodata patch the build 0 is now perfectly ok

maresb added a commit to maresb/admin-requests that referenced this pull request Jun 4, 2024
@maresb
Copy link
Contributor

maresb commented Jun 4, 2024

Thanks @OriolAbril for the correction! I must have gotten confused when flipping between the commits. I have updated conda-forge/admin-requests#1010 accordingly.

@maresb
Copy link
Contributor

maresb commented Jun 4, 2024

For clarity I have:

  • Reverted the main branch back to v0.18.0
  • Created a new branch for v0.17.x
  • Created a new build 2 for v0.17.1 with the correct dependencies.

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.

5 participants