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

BUG: fix two bugs with symlog colorbar ticks for matplotlib 3.5 #3556

Merged
merged 3 commits into from
Oct 25, 2021

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Oct 12, 2021

PR Summary

fix #3554
fix #3560

@neutrinoceros
Copy link
Member Author

Ah, this is one more place we need versioned branches to support various versions of MPL apparently

@neutrinoceros neutrinoceros marked this pull request as draft October 12, 2021 20:49
@neutrinoceros neutrinoceros marked this pull request as ready for review October 12, 2021 20:53
@neutrinoceros neutrinoceros mentioned this pull request Oct 13, 2021
42 tasks
@neutrinoceros neutrinoceros marked this pull request as draft October 13, 2021 09:14
@neutrinoceros neutrinoceros changed the title BUG: fix a bug with symlog colorbar ticks for matplotlib 3.5 BUG: two bugs with symlog colorbar ticks for matplotlib 3.5 Oct 13, 2021
@neutrinoceros neutrinoceros changed the title BUG: two bugs with symlog colorbar ticks for matplotlib 3.5 BUG: fix two bugs with symlog colorbar ticks for matplotlib 3.5 Oct 13, 2021
@neutrinoceros neutrinoceros marked this pull request as ready for review October 13, 2021 09:34
@neutrinoceros neutrinoceros marked this pull request as draft October 13, 2021 13:24
@neutrinoceros neutrinoceros marked this pull request as ready for review October 13, 2021 13:51
@neutrinoceros neutrinoceros reopened this Oct 13, 2021
@neutrinoceros neutrinoceros added backport-yt-4.0.x on-merge: backport to yt-4.0.x and removed backport-yt-4.0.x on-merge: backport to yt-4.0.x labels Oct 15, 2021
@neutrinoceros neutrinoceros marked this pull request as draft October 15, 2021 18:23
@neutrinoceros
Copy link
Member Author

switching to draft while we're stabilizing the auto backporting system

@@ -285,13 +285,18 @@ def _init_image(self, data, cbnorm, cblinthresh, cmap, extent, aspect):
)
)
elif zmax <= 0.0:
if MPL_VERSION >= Version("3.5.0b"):
Copy link
Member

Choose a reason for hiding this comment

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

Could you document why this fixes the issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

actually it's pretty hard to follow matplotlib's dev. I know they've been doing drastic refactors to the colobar objects, and I could point to the exact PRs where that happened, but their line changes are very large (+1k/-1k typically).
My guess is that this "offset" is actually an off-by-one error that slipped into yt because it was compensated by an equivalent mistake in matplotlib, which is why we need to keep it for versions older than 3.5, hence this bizarre construct. Anyway I tried to just remove this offset completely but unfortunately it broke for Matplotlib 3.4
Should I use some of this comment and bundle it with the code ?

@neutrinoceros
Copy link
Member Author

@cphyc it seems you enabled auto-merging here but didn't formally approve (which is required for auto-merge).

@cphyc cphyc merged commit 82eee9c into yt-project:main Oct 25, 2021
@neutrinoceros neutrinoceros deleted the bugfix_symlog_ticks branch October 25, 2021 17:08
@neutrinoceros
Copy link
Member Author

@meeseeksdev backport to yt-4.0.x

@lumberbot-app
Copy link

lumberbot-app bot commented Nov 6, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout yt-4.0.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 82eee9c409756f9d157573530d3438cc8da124bc
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #3556: BUG: fix two bugs with symlog colorbar ticks for matplotlib 3.5'
  1. Push to a named branch :
git push YOURFORK yt-4.0.x:auto-backport-of-pr-3556-on-yt-4.0.x
  1. Create a PR against branch yt-4.0.x, I would have named this PR:

"Backport PR #3556 on branch yt-4.0.x (BUG: fix two bugs with symlog colorbar ticks for matplotlib 3.5)"

And apply the correct labels and milestones.

Congratulation you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove Still Needs Manual Backport label once the PR gets merged.

If these instruction are inaccurate, feel free to suggest an improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants