Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: make sure all ticks show up for colorbar minor tick#12099
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Cross-ref#11004 |
Milestonig for backport, though I don't quite agree that its "Release Critical" 😉 |
ee1fb54 to4c05e62Compare| @@ -267,7 +267,8 @@ def __call__(self): | |||
| vmin = self._colorbar.norm.vmin | |||
| vmax = self._colorbar.norm.vmax | |||
| ticks = ticker.AutoMinorLocator.__call__(self) | |||
| return ticks[(ticks >= vmin) & (ticks <= vmax)] | |||
| rtol = (vmax - vmin) * 1e-10 | |||
| return ticks[(ticks >= vmin - rtol) & (ticks <= vmax + rtol)] | |||
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.
Just as a thought: Can it happen, that vmin > vmax? If so, neither the original nor the fixed code would work in that case.
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.
Yes but that gets checked and swapped inside the method before this code
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.
Confirmed that vmax and vmin get flipped if they are specified out of order. Which maybe is bad behaviour? But its what colorbar does...
marking WIP:
|
4c05e62 tof5e5750CompareThis is fine for now. New test fails master, passes with this PR. Yes, I could parameterize the test, but I don't think its so unwieldly as is. The more general issue of MaxNLocator can get its own PR where it can be properly debated. |
f5e5750 to61acc46Compare61acc46 tofbfa18eCompareDoc build seems broken, but I don't think its this PR. Something w/ stix fonts... |
Something went wrong ... Please have a look at my logs. |
FIX: make sure all ticks show up for colorbar minor tick
When things like that happen, feel free to ping me. This was an actual bug that (should) be fixed. |
@Carreau I would have gotten there eventually....Trying to get 3.0 finished off today 🤞 Could you also add a link to the logs in that error message? The question I was going to ask you first was "remind me how to find the logs...." |
I can try, but not directly, as the link changes regularly because of authentication: https://dashboard.heroku.com/apps/meeseeksbot Is the closest I can get for you, then click on papertrail. The bug is (was?) in an optimisation I was doing of reusing previously cloned matplotlib, where tring to fetch a branch fails as it is already the checked-out branch. That was becoming necessary as matplotlib takes ~70 seconds to clone which is too close the timeout where the bot get killed. |
peterkinalex left a comment
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.
lib / matplotlib / colorbar.py :
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
#12095 shows that if you expect a minor colorbar tick right at the end of a colorbar it sometimes doesn't show up due to floating point precision issues. This fixes that...
PR Checklist