8000 Update devdeps figure hashes by dstansby · Pull Request #5399 · sunpy/sunpy · GitHub
[go: up one dir, main page]

Skip to content

Update devdeps figure hashes #5399

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

Merged
merged 1 commit into from
Jun 17, 2021
Merged

Update devdeps figure hashes #5399

merged 1 commit into from
Jun 17, 2021

Conversation

dstansby
Copy link
Member

I think some of the devdeps figure hashes need updating for recent coordinate fixes, so here's an empty commit to see if the tests pass or the hashes do indeed need updating.

@ayshih
Copy link
Member
ayshih commented Jun 11, 2021

I think there will need to be a bisection to look for some change in Matplotlib. PR #5395's run of devdeps from two days ago does not show any non-colorbar differences in the figure tests:
https://69329-2165383-gh.circle-artifacts.com/0/.tmp/py37-figure-devdeps/figure_test_images/fig_comparison.html

@dstansby
Copy link
Member Author

I'd be tempted to say the differences are all so small as to be not a problem. Looking at the commit log (https://github.com/matplotlib/matplotlib/commits/master), it might be matplotlib/matplotlib#20363, but MPL didn't change any of their test images in that PR, so it might be an interaction with that and WCSAxes (at a very rough guess)>

@ayshih
Copy link
Member
ayshih commented Jun 11, 2021

It looks like they relaxed a tolerance for an image check.

Yes, we could simply update our hashes, but that Matplotlib PR may also be responsible for this contour craziness in #5398:
result
I don't know whether fixing this plot would require an upstream fix/revert to Matplotlib, but if it does, we may end up reverting the hashes in short order.

@dstansby
Copy link
Member Author

Zooming into one of the figure tests that changed, the lines are still in the same place. So I think this is probably some sort of change in the aliasing of paths, and it's safe to change the hashes and merge once #5398 is merged.

@dstansby dstansby force-pushed the fig-dev-hash branch 3 times, most recently from cc6fb22 to 92e7a18 Compare June 17, 2021 17:21
@dstansby dstansby marked this pull request as ready for review June 17, 2021 17:23
@dstansby
Copy link
Member Author

This should be good to go now - I've confirmed that all the changes are just in the way lines are drawn, and not in their location.

@dstansby dstansby added Needs Review Needs review(s) before merge. backport 3.0 No Changelog Entry Needed Skip all changelog checks. labels Jun 17, 2021
@dstansby dstansby added this to the 3.0.1 milestone Jun 17, 2021
@nabobalis nabobalis merged commit 58daf11 into sunpy:main Jun 17, 2021
@sunpy-backport
Copy link

The backport to 3.0 failed:

Commits ["6fd711369afbe0ebd2ee0c67e048be0df3275d06"] could not be cherry-picked on top of 3.0

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport 3.0
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick 6fd711369afbe0ebd2ee0c67e048be0df3275d06
# Create a new branch with these backported commits.
git checkout -b backport-5399-to-3.0
# Push it to GitHub.
git push --set-upstream origin backport-5399-to-3.0
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is 3.0 and the compare/head branch is backport-5399-to-3.0.

@dstansby dstansby deleted the fig-dev-hash branch June 17, 2021 18:25
@nabobalis nabobalis added the Still Needs Manual Backport This PR needs manually backporting. label Jun 17, 2021
@nabobalis nabobalis removed the Still Needs Manual Backport This PR needs manually backporting. label Jun 18, 2021
@dstansby dstansby removed the Needs Review Needs review(s) before merge. label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Entry Needed Skip all changelog checks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0