Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Make sticky edges only apply if the sticky edge is the most extreme limit point#28393
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
I'm a bit confused about the implementation (from a super quick skim). The intent was definitely that a sticky edgecan lead to slightly cropping an artist which barely goes beyond it (because that was necessary for streamplots, see#13980). I realize this can cause problems as mentioned here; perhaps the better fix is to actually clip streamplots as needed; but then perhaps a large chunk of the code above (around the comment explaining |
hmmm... my other thought is we could make it within a tolerance of thewidth rather than of the absolute value If streamplots are negatively affected by this change, we did not get a test failure. That was not the usecase I was looking at though, so I'm not sure. |
Ah, the source of the problem is that the "bottom" of the bar is sticky, but it is sticking to the top of the axes not the bottom (right instead of left) and when the bar is small relative to its position we do not "escape" the sticky. |
Agreed with@tacaswell's interpretation. I would still guess the right fix is to remove the isclose(), only allow exact matches, and fix (via clipping) streamplot accordingly. |
I think I understand this PR now and it is a better solution than what I was thinking of doing. I suspect that we could also fix this by coming up with a better way to compute |
Can we try to completely remove tol? |
Kyle reports trying that and it broke a lot of image tests. He also had concerns that doing so might snow ball into a re-write of the sticky system which is not something we should do in a micro release. |
Sure. I don't really have the time to look into this right now anyways... |
74c7f9a intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
…cky edge is the most extreme limit point
…393-on-v3.9.xBackport PR#28393 on branch v3.9.x (Make sticky edges only apply if the sticky edge is the most extreme limit point)
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Closes#28223
Essentially sticky edges has a tolerance value, but that value should be treated as single ended, rather than double ended.
That is, if the data extends beyond the sticky value, then normal margins, etc should be employed, and the sticky value is only used when the unpadded values areinside of the sticky limit.
This only happens when the values are within a relatively tight tolerance (~1e-5 relative value).
As such it was first noticed with date values, but that is not actually necessary for reproduction of the effect.
Here is the test image prior to the change, note that only one of the two bars are shown:
Here it is after the change, note that one side is stickied (no margin added), but the other side has the margin added as normal:
PR checklist