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

TST: (temporary) pin scipy in image tests #4541

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

neutrinoceros
Copy link
Member

PR Summary

Context: #4540

Scipy 1.11 was released 16hrs ago, so it's my number one suspect at the moment.
This PR is an attempt to narrow it down.

@neutrinoceros
Copy link
Member Author

Seems like this indeed fixes image tests, though it's not clear wether the problem is a pure regression in scipy, or an incompatibility in cartopy. Until we know who to upstream this discussion to, I advise not the merge this.

@neutrinoceros neutrinoceros added bug tests: running tests Issues with the test setup labels Jun 26, 2023
@neutrinoceros
Copy link
Member Author

Code inspection narrows it down to scipy/scipy#18502

@neutrinoceros
Copy link
Member Author

neutrinoceros commented Jun 26, 2023

I added a link to the upstream issue SciTools/cartopy#2199

AFAIC this may now be merged to unblock other PRs.

@neutrinoceros neutrinoceros marked this pull request as ready for review June 26, 2023 19:05
@neutrinoceros neutrinoceros linked an issue Jun 26, 2023 that may be closed by this pull request
@neutrinoceros neutrinoceros changed the title TST: try pinning scipy in image tests TST: (temporary) pin scipy in image tests Jun 26, 2023
Copy link
Contributor

@chrishavlin chrishavlin left a comment

Choose a reason for hiding this comment

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

Looks good, but can you open a new issue to track the cartopy issue? Or not close the existing one until we see some discussion upstream? I don't think we want to pin scipy in yt's dependencies, since this bug will only affect those with the latest scipy and using cartopy (so this PR is totally sufficient as is)... but we might want to pin cartopy to the newest version once they release a fix for the issue.

@neutrinoceros
Copy link
Member Author

Fair point, let's unlink the issue for now

@neutrinoceros
Copy link
Member Author

The discussion seem to be headed to a compatibility fix in cartopy with their next release, so things are looking good for this patch to play its role and remain a temporary workaround, I'll merge it so other PRs can move forward.

@neutrinoceros neutrinoceros merged commit 5a62e5c into yt-project:main Jun 27, 2023
@neutrinoceros neutrinoceros deleted the exp_pin_scipy branch June 27, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tests: running tests Issues with the test setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants