-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Updated numpy to 1.22 in CIs #105
Conversation
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 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted in python-pillow/Pillow#5860 this is (AFAIU) just for testing, but just to avoid misunderstandings: this change doesn't do anything to the current CI and should not be merged.
Oh? I'm totally fine if this PR is rejected, but I would have thought it was better to test with newer versions of numpy. |
You were changing things in migrators, which (aside from not being the right way to do it) would need a "rerender" of the feedstock to actually influence anything. However, it's almost certainly not correct to build against numpy 1.22, which translates into a hard lower bound for runtime. Conda-forge takes care of this by having a global pinning at build time, but a newer-than-build-version at runtime, i.e. 1.22 will already be used. However, I don't even see numpy in the list of dependencies here. As far as I can tell from upstream, it is an optional dependency that will be used when available? |
If you tell me what you're trying to achieve, I might be able to help... (it's not obvious to me from your commits) |
Just using trial and error to try and find some hint as to how to resolve the failure. If you would rather find a solution yourself, or if running your CI many times is a problem, I can stop. |
I appreciate your help, and running CI is not a problem. Your changes just seem a bit... inexperienced with the conda-forge infrastructure, hence my offer to help. |
As a high-level view, you should (almost) never have to edit anything outside the |
That code hasn't changed since python-pillow/Pillow#4794, over a year ago, so not recently no.
I recognise that my changes aren't solutions. Pillow 8.4.0 previously passed in this repository, but now it is failing. So the cause is some other dependency that has upgraded in the meantime. My thinking was to try and figure out what the dependency was that changed, so that then the root cause of the problem could be identified.
If you don't think this needs to be upstreamed, then ok, sure, my investigation into the root cause can be stopped. Patching should absolutely solve the immediate problem. |
That is a more tractable problem. We can have a look at the last passing CI run and the current ones, and then compare the environments (even with something as basic as |
I meant whether the code that does the comparison leading to:
had been changed recently... |
I think this is the issue, tbh. python-pillow/Pillow#5776 removed |
And to answer your other question, the check code hasn't changed in two years. |
I saw that it's coming from another module, but it still seems at least possible to me that the removal of Following your lead, To be very honest, I cannot discern what the extra quotes for py37&py38 should do, but it seems to go back to python-pillow/Pillow#2517. |
Thanks a lot for the patch! I picked up a rebased version of this into #104 |
Thanks. From the logs there:
(taking into account So AFAICT, |
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)I presume most of the above checklist is not relevant to this PR, since I'm not modifying the package itself in any way. This is just a suggestion to improve the automatic testing.