Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Cleanup _mathtext internal API#22204
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9bc7667
toe3236e9
Comparecad2d3b
tobf96d42
CompareIs |
Good catch, it's become unused per#18722; removed it. |
0129d3a
toa96df50
CompareMaybe/probably shouldn't go here, but I've just stumbled upon Not sure if the file is generated or hand-edited though. (If you do not think it should go here, I saw it while planning to go through it, so I can get rid of it.) Btw, as you seem to have a good idea about the |
Thanks for the ping, I commented on your two PRs. missing-references.json gets rautoegenerated with by building the docs with |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
rebased. |
Uh oh!
There was an error while loading.Please reload this page.
Edit: also removed the unused |
I think that matplotlib/lib/matplotlib/_mathtext.py Lines 1611 to 1623 in6eba0af
|
Good catch, removed. |
oscargus commentedFeb 17, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Restarting CI. (Edit: that didn't restart all tests...) |
It has never been used, and the implementation was in fact buggy (if onecalls `shrink` many times beyond num_size_levels, `.size` would continueto decrease while `.width` would not; if one then calls `grow`, `.size`and `.width` would then both increase, but get out of sync with oneanother). I can't find a reference to grow() in the TeXbook or "TeX theprogram" either(?)
None of the MathtextBackends actually accesses the .glyph_nameattribute anymore, and they are the only public API that can interactwith the glyph infos returned by the Fonts classes (and there is nopublic API to register new MathtextBackends).
Backends that need to track what characters have been used (pdf/ps, forsubsetting purposes) already have their own logic to do so, so don't doit in mathtext as well.Changes in Fonts are only touching private API.Changes in the call signature of MathtextBackendAgg/MathtextBackendPathlook like they are touching public API, but it is in fact impossible foran external user to construct a valid `box` parameter (this requiresclasses defined in `_mathtext`), so `get_results` is effectively privateAPI from a caller PoV. It is also impossible to register newMathtextBackends without touching private API.Thus, the only difference is in the tuple returned byMathtextBackendAgg, which now has one fewer element. Because the returnvalue is a tuple which is unpacked at the call site, it is not possibleto emit a proper deprecation warning (one would need to know whether theresult is being unpacked to 6 or 7 values). Callers can work aroundthis by e.g. unpacking the last item into `*_`. Note that because thisis only for MathtextBackendAgg (i.e., a raster backend), it is unlikelythat callers actually care about the used_characters info at all.
We can just inline the calls directly to the backend itself at the soleuse site (in _parse_cached). The width, height, and depth attributes onFonts are never used either.
It doesn't need to be a nested class.
This is never called, is on an internal class, and the attribute itaccesses doesn't exist as it was removed inmatplotlib#22204.
This is never called, is on an internal class, and the attribute itaccesses doesn't exist as it was removed inmatplotlib#22204.
Uh oh!
There was an error while loading.Please reload this page.
They are split into individual commits
Remove _mathtext.Node.grow.
It has never been used, and the implementation was in fact buggy (if one
calls
shrink
many times beyond num_size_levels,.size
would continueto decrease while
.width
would not; if one then callsgrow
,.size
and
.width
would then both increase, but get out of sync with oneanother). I can't find a reference to grow() in the TeXbook or "TeX the
program" either(?)
Stop computing glyph_name, postscript_name in mathtext font handling.
None of the MathtextBackends actually accesses these attributes anymore,
and they are the only public API that can interact with the glyph infos
returned by the Fonts classes (and there is no public API to register
new MathtextBackends).
Remove Fonts.{set_canvas_size,width,height,descent,get_results}.
We can just inline the calls directly to the backend itself at the sole
use site (in _parse_cached). The width, height, and depth attributes on
Fonts are never used either.
Remove used_characters tracking in mathtext.
Backends that need to track what characters have been used (pdf/ps, for
subsetting purposes) already have their own logic to do so, so don't do
it in mathtext as well.
Changes in Fonts are only touching private API.
Changes in the call signature of MathtextBackendAgg/MathtextBackendPath
look like they are touching public API, but it is in fact impossible for
an external user to construct a valid
box
parameter (this requiresclasses defined in
_mathtext
), soget_results
is effectively privateAPI from a caller PoV. It is also impossible to register new
MathtextBackends without touching private API.
Thus, the only difference is in the tuple returned by
MathtextBackendAgg, which now has one fewer element. Because the return
value is a tuple which is unpacked at the call site, it is not possible
to emit a proper deprecation warning (one would need to know whether the
result is being unpacked to 6 or 7 values). Callers can work around
this by e.g. unpacking the last item into
*_
. Note that because thisis only for MathtextBackendAgg (i.e., a raster backend), it is unlikely
that callers actually care about the used_characters info at all.
Expire rename_parameter in _mathtext.
Lift Parser.State to toplevel as standalone class.
It doesn't need to be a nested class.
Remove now unused StandardPsFonts, latex_to_{standard,cmex}, use_cmex.
Remove unused SubSuperCluster.
PR Summary
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).