Repository navigation
Reverted a previous change to artist transform setting. Fixes legend bug. - #1176
Conversation
|
That's really strange. I would have considered the "t is not None" test as being superior to blindly setting the boolean to True. Maybe some code somewhere is fiddling around with the _transform attribute directly when it shouldn't? |
Me too! The |
|
Thanks for finding this and tracking this down. I think what's not clear is that a transform of None is set, that's equivalent to an identity transform, and should be treated as an explicit transform having been set as far as the offsetbox is concerned. I think this change is correct, though I haven't run it against the test suite to see if it produces additional failures. |
You were right to have your suspicions. I do get failures as a result of this. Back to the drawing board... might not find a solution to this until tomorrow. |
|
Ok. I have solved this issue the proper way. I did a lot of reading of the legend code (phew, its not particularly easy to follow), found the problem (the transforms weren't being set correctly) and fixed a couple of typos. Unfortunately, my editor had a setting enabled to fix the whitespace problems automatically, which adds a little bit of noise to this PR. It wouldn't be unreasonable to ask me to revert them, but I am keeping them for now as they do improve the overall readability of the code (and hence my sanity while I was debugging this). |
@mdboom any advice on how best to go about this? |
|
Maybe the best/easiest thing to do is to just take the text out -- the legend should layout good enough for the purposes of the test without it. |

On master, the following code fails to put the markers in a legend in the correct place:
It turns out this is a result of a previous pull request of mine (#1090). This PR fixes this regression (and adds a suitable test). I will milestone this for 1.2 as it is a pretty critical bug fix.