Update watermark example #25689
Update watermark example #25689
Conversation
|
I'm all for updating the logo, but I think it's much clearer what figimage is doing on the old one because the position is hard coded. I don't know that enough folks want the watermark in the middle like that (vs the edge) that it pays to show the magic to always have it in the middle. Alternatively, maybe add a comment on how these two lines always center the image regardless of figsize? |
| image_size = (542, 130) | ||
| nx, ny = fig.get_size_inches() * fig.dpi | ||
|
|
||
| fig.figimage(im, (nx - image_size[0]) / 2, (ny - image_size[1]) / 2, zorder=3, alpha=.7) |
There was a problem hiding this comment.
I think figimage is buggy, so probably shouldn't be used at all? #23399
On my hi-dpi screen this is half size:
| image_size = (542, 130) | |
| nx, ny = fig.get_size_inches() * fig.dpi | |
| fig.figimage(im, (nx - image_size[0]) / 2, (ny - image_size[1]) / 2, zorder=3, alpha=.7) | |
| axin = fig.add_axes([0, 0, 1, 1]) | |
| axin.axis('off') | |
| axin.imshow(im, alpha=0.7) |
There was a problem hiding this comment.
I'm a bit torn here. Overlaying an Axes and plotting the image into that feels like a hacksish workaround workaround. OTOH figimage has currently it's deficits. (Not only the dpi issue, but also API wise: One would want a proper positioning logic (horizontal/vertical align, bbox_to_anchor, sizing).
What I additionally don't like about the Axes/imshow approach is that you get sampling artifacts unless you make sure the image is exactly the same size as the figure.
I will rethink how to approach this. (and also consider @story645's remarks)
There was a problem hiding this comment.
What I additionally don't like about the Axes/imshow approach is that you get sampling artifacts unless you make sure the image is exactly the same size as the figure.
I'm not sure I understand what you are referring to here.
I don't personally consider placing Artists in an Axes rather than on a Figure a "hack". Almost all our artists are created on Axes, and I don't think that there is much to be gained by making figure-equivalent versions of most of those methods.
I do consider axin.axis to be a bit clunky, but so far as I know that is the one-line way to make the axes disappear.
If we were to add an API, it might be fig.figure_axes which just does ax = fig.add_axes([0, 0, 1, 1]); ax.axis('off') to give a figure-wide canvas.
There was a problem hiding this comment.
What I additionally don't like about the Axes/imshow approach is that you get sampling artifacts unless you make sure the image is exactly the same size as the figure.
I'm not sure I understand what you are referring to here.
figimage only gets the anchor point an then draws the image 1:1 on the pixels -> every image pixel appears 1:1 in the figure. In constrast, for imshow the image is scaled to the size of the Axes. This scaling creates artifacts. While scaling can be a nice feature, there should be a way to have images rendered 1:1 on the pixels.
There was a problem hiding this comment.
This is a watermark example - I would have thought we would want the watermark to rescale if the figure changes size?
Figimage indeed does what it says, which is "Add a non-resampled image to the figure.", but the example doesn't properly query the dpi or create a figure that is the right size for the logo png. As a general sniff test, if an example needs fig.dpi it's likely to be incorrect responding to figure size changes or screen resolutions.
Note: this replaces mpl-data/sample_data/logo2.png with the current logo. I think that's ok and we do not guarantee content stability of that data (internally, it's only used for the watermark example). But we could also add it as logo3.png at the cost of having 22kB more data.
c08d4dd to
b18beae
Compare
|
To avoid any discussion on positioning and scaling behavior, I've reverted to a fixed position. This is now only a stylistic update that uses the current logo (and more realistic looking data - who would plot girant circle markers on a sine?).
|
Yes. There are multiple uses of |
|
opened an imread issue #25914 |
|
Strange that this old logo doesn't match the one in https://matplotlib.org/stable/users/project/history; I wonder when it was in use? And should it be moved there? |
If you look through https://github.com/matplotlib/matplotlib/commits/main/doc/_static/logo2.png You'll see Oct 14 2008 https://github.com/matplotlib/matplotlib/blob/5a4bbf53f50a8be6e231b5e0d02aacf9ad3bffa5/doc/_static/logo2.png Oct 15 2008 https://github.com/matplotlib/matplotlib/blob/a73a025260a4446e0c5d5344aad39180f4e2182b/doc/_static/logo2.png Oct 16 2008 https://github.com/matplotlib/matplotlib/blob/1cc29a36ef75e8b80e0f2a19e0d804ffbd5b447f/doc/_static/logo2.png The first two were only there for a day each, so don't count as official logos. |
|
I take the liberty to merge based on @story645's approval. |
Note: this replaces mpl-data/sample_data/logo2.png with the current logo. I think that's ok and we do not guarantee content stability of that data (internally, it's only used for the watermark example). But we could also add it as logo3.png at the cost of having 22kB more data.
old: https://matplotlib.org/3.7.1/gallery/images_contours_and_fields/watermark_image.html#sphx-glr-gallery-images-contours-and-fields-watermark-image-py
new: