-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Pass gid to renderer #15087
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass gid to renderer #15087
Conversation
Fix seems simple enough. Should be able to write a test that looks for a specified gid in resulting SVG output, no? |
I test it using this code: import matplotlib.pyplot as plt
FIGURE_ID = "the_figure"
FIGURE_PATCH_ID = "the_figure_patch"
fig = plt.figure(figsize=(15, 9))
# fig.canvas.draw()
fig.set_gid(FIGURE_ID)
fig.patch.set_gid(FIGURE_PATCH_ID)
print(fig.get_gid())
print(fig.patch.get_gid())
fig.savefig('id_test.svg', format='svg') And the output in svg file is: <g id="the_figure">
<g id="the_figure_patch"> |
ce207cd
to
fdcce93
Compare
I wrote a test and it seems almost half of the tested artists do not pass on the gid. So what will need to be done here is to systematically find all cases. Hence marking as work in progress for now. |
Also before working more on this, can I have opinions on whether you would consider this as a bug fix or an API change (in the sense of, might we annoy people more by fixing this than we help the few that need it?) |
Does |
fdcce93
to
d0c6f0c
Compare
0f98cbc
to
1fcb7fc
Compare
I handled all missing gids now. The test is a bit montrous; at least it makes sure a good cross section of artists are tested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test feels a bit cumbersome, but does serve the purpose.
buf = fd.read().decode() | ||
fd.close() | ||
|
||
def include(gid, obj): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps has_gid()
or gid_is_set()
(it's not totally clear what "include" means here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really literally, whether or not to include an object in the test. (Not all objects in the figure actually appear in the svg, so we need to exclude them). I've added a docstring to it.
1fcb7fc
to
2291561
Compare
|
||
fig.canvas.draw() | ||
|
||
counter = [0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
counter = itertools.count()
and use next(counter)
in assign_gid
(or rather, enumerate() is probably enough:
for idx, obj in enumerate(fig.findobj(...)):
gid = f"...{idx}"
gdic[gid] = obj
obj.set_gid(gid)
)
ax3 = fig.add_subplot(133, projection="3d") | ||
ax3.plot([1, 2], [1, 2], [1, 2]) | ||
|
||
fig.canvas.draw() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need the draw() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I don't draw it, the ticks will not be the ones that are exported, because the ticker only creates them when drawing the figure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, of course.
2291561
to
9f0b2e9
Compare
fig.savefig(fd, format='svg') | ||
fd.seek(0) | ||
buf = fd.read().decode() | ||
fd.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could just write:
with BytesIO() as fd:
fig.savefig(...)
buf = fd.getvalue() # no need to seek() :)
Hopefully this will resolve the code coverage issue.
Thanks @ImportanceOfBeingErnest ! |
PR Summary
Previously the figure's
gid
would not be passed to the renderer.Fixes the issue brought up in https://stackoverflow.com/questions/57563180/fail-to-set-figure-gid-in-python-matplotlib
Previously:
With this PR:
Also, do the same with axis, ticks and legend and all other artists.
I did not perform a complete search on whereelse it might be missing.PR Checklist