Repository navigation
Cleaning up the Formatter API #5787
Description
Still on my offset-text rewrite spree...
Currently, the formatter API (in charge of picking the strings for the tick labels, axis offset and cursor position in the status bar) is a bit messy: the public methods available are
- Formatter.__call__(self, val, idx): computes the label texts
- Formatter.format_data(self, val): not publicly used
- Formatter.format_data_short(self, val): computes the cursor position text
- Formatter.get_offset(self): returns the offset text
- Formatter.set_locs(self, locs): required for computing the labels
- Formatter.fix_minus(self, s): overriden only by ScalarFormatter to use a unicode minus
FixedFormatter also provides set_offset_string.
The commonly used ScalarFormatter and LogFormatter also provide pprint_val (with different signatures, and not publicly used).
ScalarFormatter also provides a couple of getter/setters (for useOffset, useLocale, scientific, powerlimits); LogFormatter provides base and label_minor as setters for instance variables.
LogFormatterExponent and LogFormatterMathtext are variants of LogFormatter.
EngFormatter also provides format_eng (not publicly used).
There are also a bunch of instance attributes which are public but probably intended as private.
I propose simplifying the API to the following:
- Formatter.get_label_text(self, val: float, idx: int): computes the label texts
- Formatter.get_cursor_text(self, val: float): computes the cursor position text
- Formatter.get_offset_text(self) and Formatter.set_offset_text(self, s: str): get/set the offset text
- Formatter.set_locs(self, locs: array-like): because we need it
- Formatter.set_uselocale(self, b: bool): because there's no reason only ScalarFormatter has it
(the new method names can be made to fallback on the old method names with a deprecation warning in the base class)
and drop or make private basically everything else:
- fix_minus (let's first check if any backend doesn't support unicode)
- pprint_val/format_eng
- the various instance attributes which should have been private to start with (keeping: scientific, powerlimits, base, label_minor, using getter/setters)
and fold the LogFormatterExponent and LogFormatterMathText classes into options for the LogFormatter class.
Thoughts?
Activity
tacaswell commented on Jan 2, 2016
From 10k feet looks good.
Keep in mind the coming conversion to traitlets which is going to deprecate most of the getter/setter methods (set_locs and set_uselocale both fall in that camp, maybe offset_text as well). It might be better to use names without get for the public methods. Maybe Formatter.for_label and for_cursor
I have a preference for more simpler functions/classes over fewer more complex functions/classes. Having attributes/arguments that affect drawing conditional on the value of some other attribute/argument makes the API more frustrating to use.
tacaswell commented on Jan 2, 2016
Also going back and forth on if this should be lumped in with 2.0 style changes or 2.1. Tagged this as 2.1, but if these changes make the style changes to the offset text easier to do I am in favor of re-implementing the old API in terms of the new, using the new everywhere internally and starting the deprecation cycle on the old in 2.1 (to be removed in 2.3).
anntzer commented on Jan 3, 2016
What's the status of the switch to traitlets? (Specifically, is it certain that there will be a switch?) Given that the Formatter class is actually fairly self-contained, would it be OK for me to directly write a traitlets-based API? (I don't want to maintain two compatibility layers.)
I would suggest Formatter.as_label_text and Formatter.as_cursor_text, perhaps.
Do you mean that you'd rather keep LogFormatterExponent and LogFormatterMathText out? I personally think that ax.get_xaxis().get_major_formatter().use_mathtext = False would be easier to use than having to import a new class and swap it in with set_major_formatter for that purpose.
tacaswell commented on Jan 3, 2016
We have not hit a major technical issue with traitlets yet. Unless there is
something like a show stopping unresolvable speed regression that will be
one of two main features for 2.1 (the other being the gui classes overhaul).
On Sat, Jan 2, 2016, 20:18 Antony Lee notifications@github.com wrote:
mdboom commented on Jan 4, 2016
It would be nice to have this on 2.0 (I think), which won't have Traitlets. So, unfortunately, if we agree it should be on 2.0, we will have to do a non-Traitlets implementation now and a Traitlets one later. In our experience with Traitlets so far, though, this won't mean maintaining two code paths, but simply replacing to use Traitlets.
tacaswell commented on Jan 4, 2016
A pattern we used else where is to use the 'old-school' property model to
get both the getter/setter api + property API.
On Mon, Jan 4, 2016 at 10:19 AM Michael Droettboom notifications@github.com
wrote:
github-actions commented on Mar 20, 2023
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!
anntzer commented on Mar 20, 2023
This is somewhat outdated, but we could still at least consider deprecating (or rather making private) format_data, which is not actually used anywhere other than as a internal helper, as well as set_locs (per #13323).
github-actions commented on Mar 22, 2024
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!