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 for matplotlib 3.5 + symlog + linthresh value outside data range #3565

Merged
merged 1 commit into from
Oct 25, 2021

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Oct 13, 2021

PR Summary

fix #3564

Here's the results from #3564's script with this branch with matplotlib 3.4 or 3.5
mpl_3 5 0rc1__Slice_z_my_fake_density

So we even gained minor ticks for matplotlib 3.4, which alleviate most of the problem I reported in #3537 🎉
opening as a draft for now, will open for review if CI stays green

@neutrinoceros neutrinoceros changed the title BUG: fix a bug with matplotlib 3.5 wherre using a linthresh value outside the actual data range would produce artifacts in colorbars BUG: fix for matplotlib 3.5 + symlog + linthresh value outside data range Oct 13, 2021
@neutrinoceros neutrinoceros marked this pull request as ready for review October 14, 2021 07:33
@neutrinoceros neutrinoceros mentioned this pull request Oct 14, 2021
42 tasks
@neutrinoceros neutrinoceros marked this pull request as draft October 16, 2021 08:09
@neutrinoceros
Copy link
Member Author

switching to draft while we're stabilising the auto backport system

elif zmin * zmax > 0 and cblinthresh < min_abs_val:
warnings.warn(
f"Symlog linear threshold value {cblinthresh} is outside "
f"the data range ({zmin}, {zmax}). Switching to log norm."
Copy link
Member

Choose a reason for hiding this comment

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

Note: the warning will be slightly erroneous if both zmin and zmax are negative. Maybe change it for something like

Suggested change
f"the data range ({zmin}, {zmax}). Switching to log norm."
f"the absolute data range ({abs(zmin)}, {abs(zmax)}). Switching to log norm."

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, you have a point, but your suggestion has a drawback too when values are both negatives because their absolute values are now in reversed order. I'll try to improve over it.

Copy link
Member Author

Choose a reason for hiding this comment

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

better now ? (sorry, I forced pushed again, but only the warning message was changed)

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I clicked the auto-merge button ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you !

…side the actual data range would produce artifacts in colorbars
@cphyc cphyc enabled auto-merge October 25, 2021 14:05
@cphyc cphyc merged commit ecf9a82 into yt-project:main Oct 25, 2021
@neutrinoceros neutrinoceros deleted the hotfix_3564 branch October 25, 2021 15:22
@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 ecf9a8253b3386b0a9d87e9f99c85ec337d3fbfa
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #3565: BUG: fix for matplotlib 3.5 + symlog + linthresh value outside data range'
  1. Push to a named branch :
git push YOURFORK yt-4.0.x:auto-backport-of-pr-3565-on-yt-4.0.x
  1. Create a PR against branch yt-4.0.x, I would have named this PR:

"Backport PR #3565 on branch yt-4.0.x (BUG: fix for matplotlib 3.5 + symlog + linthresh value outside data range)"

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
Development

Successfully merging this pull request may close these issues.

BUG: symlog colorbar with linear threshold < vmin + matplotlib 3.5
2 participants