In imsave, let pnginfo have precedence over metadata. - #15434
Conversation
This is consistent with the documented behavior ("If the 'pnginfo' key
is present, it completely overrides *metadata*, including the default
'Software' key.") and also with the behavior of
FigureCanvasAgg.print_png.
anntzer
added
the
Release critical
|
Is #13092 really the relevant issue here? |
| "RGBA", pil_shape, rgba, "raw", "RGBA", 0, 1) | ||
| if format == "png" and metadata is not None: | ||
| if (format == "png" | ||
| and metadata is not None and "pnginfo" not in pil_kwargs): |
There was a problem hiding this comment.
Silently ignoring metadata if pnginfo is present does not feel right. Can we raise or warn in that case, or are there subsets of metadata that may still be needed?
There was a problem hiding this comment.
print_png (used by savefig("foo.png") has had this behavior ever since pil_kwargs is supported (which is 3.1), and no one has complained about it so far (it probably hasn't been too much used yet in the wild, though); I'd rather keep both places in sync.
Also a verb is missing in your sentence ("Can we ???").
There was a problem hiding this comment.
Fixed the missing verb. I'd be happy to warn in both places. Raising would now be a breaking change.
I wouldn't expect too much complaint. It's not widely used, it will only happen if you provide both parameters. It might actually have affect users without them realizing it.
There was a problem hiding this comment.
sure, added a warning in both places.
|
edited: the correct original PR was #13902. |
e7e6343 to
9e9c5d1
Compare
3 participants
This is consistent with the documented behavior ("If the 'pnginfo' key
is present, it completely overrides metadata, including the default
'Software' key.") and also with the behavior of
FigureCanvasAgg.print_png, cf
matplotlib/lib/matplotlib/backends/backend_agg.py
Lines 522 to 524 in c9f292f
The feature came in in #13902 which is not in any released version so far, so it would be nice if this was fixed before the final 3.2 release. I will prepare a separate PR for master.
PR Summary
PR Checklist