Fix scatter legend with numpoints == 1 #6883
Conversation
Fix incinsistency between Agg and vector backends
| @@ -293,13 +293,14 @@ def draw_path_collection(self, gc, master_transform, paths, all_transforms, | |||
| """ | |||
| path_ids = [] | |||
| for path, transform in self._iter_collection_raw_paths( | |||
| master_transform, paths, all_transforms): | |||
| master_transform, paths, all_transforms, offsets): | |||
There was a problem hiding this comment.
wouldn't the transform for the offsets need to be applied first?
There was a problem hiding this comment.
In this case the offsets are independent of the transforms (the latter used primarily for marker size). Way down underneath they are combined for the Agg backend. The vector backends can't however, because those formats would scale the stroke width as a result.
But maybe I don't understand your question/concern?
There was a problem hiding this comment.
I honestly don't know if there is a problem or not. Only that typically elsewhere in mpl, we have the transforms for the offset independent of the transform for other aspects of an artist, I think. So, to see the offsets passed into a function, but not the transform for the offset, I start wondering if this might be subtly broken?
|
Is it just me, or is the dot a little low in the legend? |
I saw that too, but it's really just a matter of pixel snapping not being as advantageous with the slightly different size. Not sure what we can do about that... |
| @@ -356,7 +357,7 @@ def draw_gouraud_triangles(self, gc, triangles_array, colors_array, | |||
| self.draw_gouraud_triangle(gc, tri, col, transform) | |||
|
|
|||
| def _iter_collection_raw_paths(self, master_transform, paths, | |||
| all_transforms): | |||
There was a problem hiding this comment.
I do not understand, this does not use offsets in the function?
|
This probably needs an api_changes entry an it could break third-party backends. |
fc13922 to
6f03d53
Compare
|
power-cycling to see if failures go away. |
|
So, the only thing left that has me concerned is the point @tacaswell brought up. If I read this all correctly, this does have the potential to break 3rd-party backends. Even worse, it would be hard for such a backend to support multiple versions of matplotlib because the new argument is in the middle of the call signature. Does it make sense to at least provide some example code for 3rd party backends to copy-n-paste into their own code to paper over this change? |
|
@WeatherGod I think the change that broke things (adding an |
|
No, I am talking about the new arguments to |
| return (N + Npath_ids - 1) // Npath_ids | ||
|
|
||
| def _iter_collection(self, gc, master_transform, all_transforms, | ||
| path_ids, offsets, offsetTrans, facecolors, | ||
| paths, path_ids, offsets, offsetTrans, facecolors, |
There was a problem hiding this comment.
@WeatherGod Right, I noticed the new arg at the end, but not the one in the middle.
|
So can this be closed what with #7214 being merged instead? |
|
Yes. |
Fix inconsistency between Agg and vector backends
Fix #6845.