-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Replace MathtextBackend mechanism. #22506
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
Conversation
6f9692a
to
57e7898
Compare
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.
The [source]-link for VectorParseResult
and RasterParseResult
does not appear to be working (as opposed to get_unicode_index
). Not sure, but maybe something like VectorParseResult.__module__ = __name__
should be done? (Only difference I could spot).
The documentation for those are a bit sporadic "alias for field 0" etc, but maybe more a problem that it says that than it is not explicitly documented, if anything. (Feel free to ignore.)
Overall, I think it makes sense, but maybe one can also add a bit more information in the deprecation notice along the lines "MathTextParser.parse() will instead return VectorParseResult or RasterParseResult." so that people seeing the deprecation warning and checking the notes will get some help into what to do instead.
Better docs could be nice, but previously the return type (well, the fields of the returned tuple) of MathTextParser.parse were not documented at all, so I think this is already an improvement; further documentation can be done later.
There is no deprecation; the Parse results (now renamed to VectorParse/RasterParse) are fully backcompatible with the old ones (they inherit from tuple). |
But there is?
Not sure how they worked out, but they were public in a public module, so in case someone use them directly (for unknown reasons), there is currently no information about how to replace them. If I understand correctly, it only matters if creating them directly or checking the type of the return object? So it is unlikely that anyone will really need to change anything in their code? |
While users can create MathtextBackend objects directly, there is no public API whatsoever that accepts or returns them (they are constructed internally by MathTextParser.parse, not taken as arguments or returned), nor is it possible for the end users to call their main API method (get_results), because get_results takes a Box object, which is an instance of a private class. Even calling render_glyph would be quite difficult, as the third parameter (info) is a SimpleNamespace which needs specific (undocumented) fields. So they can't do anything useful with the instance they have, and checking for that type isn't useful either. Altogether, I believe this means that just noting that the classes are gone is sufficient and more info would actually make the changelog more confusing for readers, not less; moreover, there is simply no replacement for these APIs anyways. |
rebased. |
The MathtextBackend ("MB") mechanism was previously used to let actual backends customize how they received mathtext results -- either as lists of glyphs and rectangles (for vector backends: MathtextBackendPath), or a bitmap (for raster backends: MathtextBackendAgg); in both cases, metrics are also provided. MBs also controlled font hinting. Note that the MB mechanism was not publically user-extendable (this would require touching the private MathTextParser._backend_mapping dict), so third parties could not meaningfully provide their own backends. MBs were attached to _mathtext.Fonts objects, which were central to the "shipping" stage of the parse (ship(), which converts the nested parse tree created by pyparsing into flat calls to render_glyph and render_rect_filled). This led to a slightly curious API, where the old MathtextBackendAgg.get_results() (for example) calls `_mathtext.ship(0, 0, box)` and this somehow magically mutates self -- this is because self is indirectly attached to sub-elements of box. This PR changes the implementation to instead detach output logic from Fonts (which become restricted to providing glyph metrics and related info), and makes ship() instead return a simple Output object (lists of glyphs and rects) which is itself able either to convert to a VectorParse or a RasterParse -- namedtuples that are backcompatible with the tuples previously returned by MathTextParser.parse(). (While technically these are "new" classes in the API, they are simply there to (slightly) better document the return value of MathtextBackend.parse().) In summary, this patch - removes the non-extensible MB system, - detaches output logic from Fonts objects, thus avoiding "action at distance" where `ship(0, 0, box)` would mutate the calling MB, - (weakly) documents the return value of MathtextBackend.parse(). Unrelatedly, also deprecate the unused MathTextWarning.
The MathtextBackend ("MB") mechanism was previously used to let actual
backends customize how they received mathtext results -- either as lists
of glyphs and rectangles (for vector backends: MathtextBackendPath),
or a bitmap (for raster backends: MathtextBackendAgg); in both cases,
metrics are also provided. MBs also controlled font hinting. Note that
the MB mechanism was not publically user-extendable (this would require
touching the private MathTextParser._backend_mapping dict), so third
parties could not meaningfully provide their own backends.
MBs were attached to _mathtext.Fonts objects, which were central to
the "shipping" stage of the parse (ship(), which converts the nested
parse tree created by pyparsing into flat calls to render_glyph and
render_rect_filled). This led to a slightly curious API, where
the old MathtextBackendAgg.get_results() (for example) calls
_mathtext.ship(0, 0, box)
and this somehow magically mutates self --this is because self is indirectly attached to sub-elements of box.
This PR changes the implementation to instead detach output logic
from Fonts (which become restricted to providing glyph metrics and
related info), and makes ship() instead return a simple Output object
(lists of glyphs and rects) which is itself able either to convert to
a VectorParse or a RasterParse -- namedtuples that are backcompatible
with the tuples previously returned by MathTextParser.parse(). (While
technically these are "new" classes in the API, they are simply there to
(slightly) better document the return value of MathtextBackend.parse().)
In summary, this patch
distance" where
ship(0, 0, box)
would mutate the calling MB,Unrelatedly, also deprecate the unused MathTextWarning.
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).