Skip to content

New Formatter API #5804

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

Closed
wants to merge 3 commits into from
Closed

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 6, 2016

This is a complete rewrite of the Formatter API, as suggested first in #5787.

  • A back-compatibility layer is missing.
  • Docs probably need to be updated.
  • The LogFormatter and variants classes could use some improvement. The OldScalarFormatter has been removed.
  • Assigment to Axes.fmt_{x,y}data has been removed as assignment to Axes.format_{x,y}data does exactly the same.

There are also some significant changes to the output of ScalarFormatter (which was the initial goal of this patch). It should be possible to factor them out if needed.

@tacaswell tacaswell added this to the next major release (2.0) milestone Jan 6, 2016
@tacaswell
Copy link
Member

Seeing the output, including all of the zeros looks much worse. I am not sure that I buy the argument about the offset and the ticks being the same precision as in some sense the offset has machine precision (as it is constant we get to pick as we choose).

I can not stress enough how important maintaining back compatibility is.

In the discussion before I missed your suggestion to remove __call__, that should definitely stay.

Please revert the removal of fmt_*. It is a bit redundant but there is no reason to break that API. Think how annoyed you would be if someone else broke your code over something like that.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 6, 2016

Backwards compatibility

I certainly intended to maintain backwards compatibility; however, I also chose to not implement it in the first commit specifically to be certain that I found all the places where the API is used internally (note that it is now much easier to go back, grepping format_for_tick or format_for_cursor is easier than looking for calls to formatters...). I also agree that the removal of fmt_* is a bit gratuitous, I'll put them back.

What API do you suggest for Formatter? __call__ and format_for_cursor? (that would sound reasonable to me.)

The format of the offset text

Note that the example images of the tests use particularly contrived tick values (I don't think [0.5000001, 0.5000002] is a typical x interval). If you look at a more common case, I guess that when I plot a range of [12345671, 12345678], having 0..7 + 1.2345671e7 as labels (the current case) probably looks fine to you. But if I plot [12345670, 12345678], this becomes 0..8 + 1.234567e7 and I need to count decimal points to see at what decimal place of the offset I need to add the label. And of course you can progressively make it worse ([12345600, ..], [12345000, ..], ...). For a particularly pathological case with the current settings (basically the same issue as the one you're mentioning), a range of [10000001, 10000009] currently yields labels of 0..8 + 1.0000001e7, which I don't think is particularly aesthetic either. Now, in theory the maximum number of zeros you can get is probably 15 or 16 (i.e. the precision of a float), which I agree will never look good (on the other hand I don't know what kind of real-life data that would be); so perhaps I can threshold this to say at most 6 zeros.

self._useMathText = useMathText
self.orderOfMagnitude = 0
self.format = ''
def __init__(self, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not use kwarg popping and please preserve the current API (useMathText).

@efiring
Copy link
Member

efiring commented Jan 25, 2016

Formatting suggestion: put spaces around the +, or at least before it. Maybe also a space after the x.

A back-compatibility layer is missing.  The LogFormatter and variants
classes could use some improvement.

Assigment to `Axes.fmt_{x,y}data` has been removed as assignment to
`Axes.format_{x,y}data` does exactly the same.
@anntzer
Copy link
Contributor Author

anntzer commented Jan 27, 2016

Slowly working on restoring backcompat. I think mathtext already handles spacing around operators (or at least it should?).

self._round_to = round_to

def __call__(self, x, pos=None):
def format_for_tick(self, x, pos=None):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again a much needed improvement IMHO. If we really want __call__() functionality I would just call format_for_tick() without any additional code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping __call__ is a pretty big API change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with a call with non-annotated code is that it is hard to interpret.

The obvious solution is to document carefully what the call method does. The only downside is that this documentation is not directly obvious for a derived class if that class is not documented properly.

To keep Ticker etc a callable object, either format_for_tick() method could be invoked in the call() although that might be a bit artificial thinking of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not quite follow this. The __call__ method is no different of a documentation burden than any other method and IPython's help prints it out nicely:


In [9]: ax = plt.gca()

In [10]: fmt = ax.xaxis.get_major_formatter()

In [11]: ? fmt
Type:           ScalarFormatter
String form:    <matplotlib.ticker.ScalarFormatter object at 0x7f836fcd22b0>
File:           ~/src/p/matplotlib/lib/matplotlib/ticker.py
Signature:      fmt(x, pos=None)
Docstring:
Tick location is a plain old number.  If useOffset==True and the data range
is much smaller than the data average, then an offset will be determined
such that the tick labels are meaningful. Scientific notation is used for
data < 10^-n or data >= 10^m, where n and m are the power limits set using
set_powerlimits((n,m)). The defaults for these are controlled by the
axes.formatter.limits rc parameter.
Call docstring: Return the format for tick val *x* at position *pos*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The burden is the same. I am just trying to say that if documentation is omitted - which might be the case for a quickly written and very simple derived class - the name of the method (i.e. __call__) cannot help you to understand what it does.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 23, 2016

I just had an actual use case where the reliance on Formatter.__call__ rather than a non-magic method actually made things harder for me.

Briefly, I have a figure with a very large number of subplots, so that labeling the ticks would yield a too crowded, illegible result. Instead I simply made sure that the tick spacing is consistent across the figures and wrote it as the suptitle, and set the tick labels to [].

Unfortunately, this also affects the x=... y=... status bar text when hovering with the mouse (by the way displaying "x= y=" with no values doesn't look very good either but whatever), which I'd like to keep because, well, I don't have the tick labels anymore. If we were using a normal method, then I could simply do

ax.xaxis.get_major_formatter().format_for_tick = lambda *args, **kwargs: ""

(instead of calling set_xticklabels([])). But because of the way special-methods are looked up (not on the instance), this won't work with __call__.


I guess backwards compatibility comes into play in two places here:

  1. Formatter objects should stay callable so that third party libraries/programs can call them.
    For that aspect, I think call-ability can reasonably be deprecated with a __call__ that prints a warning and delegates to format_for_tick. Is this use-case truly common?

  2. Old custom formatters should not be broken.
    I believe that even as of today, custom formatters need to inherit from the base Formatter class. Thus, e.g. with the proper metaclass magic (perhaps not even needed), classes that redefine any of __call__, format_data or format_short can be wrapped into the new API. Or the creation of a wrapper object can happen during the call to ``set_{major,minor}_formatter` too.

In a sense, it may even be a better improvement if set_major_formatter just accepted any callable, wrapping it into a suitable wrapper object for later consumption.

@QuLogic
Copy link
Member

QuLogic commented Mar 23, 2016

Briefly, I have a figure with a very large number of subplots, so that labeling the ticks would yield a too crowded, illegible result. Instead I simply made sure that the tick spacing is consistent across the figures and wrote it as the suptitle, and set the tick labels to [].

I think you may be working around this the wrong way. Use ax.tick_params(labelbottom=False, labeltop=True) to put labels on the top-most axis. Also, consider plt.subplots(..., sharex=True) to ensure the tick spacing is consistent.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 23, 2016

This won't actually work for me, because the subplots have different, non-shared x and y scales. Basically, the events I'm plotting occur at different absolute times and different absolute positions, so I'm using the ticks simply as a reference to compare their scales.

@tacaswell
Copy link
Member

T he format_xdata and fmt_xdata distinction exists to support the use case where you want the mouse over to be different from the tick labels in general.

You can use a NullFormatter which exists for this exact use case (no tick labels, but mouse over still works).

I do not find supporting that sort of per-instance monkey-patching to recreate existing functionality a compelling reason to change a long standing API.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 23, 2016

  1. By default, NullFormatter does not display values on mouse-over (e.g.: pylab_examples/scatter_hist.py, tested on 1.5.1 and master (by the way, on master, the scatter plot seems to use a slightly different color than the histograms). I'm sure you meant that I could start with a NullFormatter and set fmt_xdata, but...
  2. The fmt_xdata mechanism allows you to override the cursor text, but not the label text. So, it's a bit awkward if I want to take advantage of a non-trivial cursor text formatter that has already been implemented somewhere else. For example, I think I had suggested somewhere else (but can't find the reference right now) that the number of significant digits of the cursor text should by default be set to match the pixel size of the figure (because you can't position your cursor more precisely than that -- I actually find it quite annoying that so many non-significant digits are displayed). Then the current API would entail something like
ax.set_major_formatter(NullFormatter())
ax.fmt_xdata = <DefaultFormatterCls>.format_data_short

rather than the, IMO, simpler

ax.get_major_formatter().format_for_tick = lambda pos, i=None: ""
  1. I don't really think format_xdata and fmt_xdata do different things is a very user-friendly (or very well-documented...) API.

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.0 (style change major release) May 2, 2016
@efiring
Copy link
Member

efiring commented May 2, 2016

@anntzer, I think there will be wide agreement that the Formatter-related API and underlying code need improvement, and you are largely on track--but this needs to be thought out and discussed more, and should not hold up 2.0. Hence the change in milestone.

@anntzer
Copy link
Contributor Author

anntzer commented Nov 1, 2016

attn @NelleV

@anntzer
Copy link
Contributor Author

anntzer commented Dec 8, 2016

Note to self, once we get to revisit this issue: we could also have nearly stateless formatters, which are simply callables with the signature fmt(axis, tick), where tick can either be a list of values (in the initial case of getting tick labels) or a single value (for computing cursor data). Offset text would be handled by having the call directly modify the Axis instance.

I say nearly stateless because we want to maintain configurability (use_offset, etc.) in the state.

@QuLogic QuLogic assigned QuLogic and unassigned QuLogic May 31, 2017
@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@anntzer anntzer modified the milestones: needs sorting, unassigned Feb 15, 2018
@anntzer
Copy link
Contributor Author

anntzer commented Jun 24, 2018

Closing as bitrotted, but probably worth revisiting at some point.

@anntzer anntzer closed this Jun 24, 2018
@anntzer anntzer self-assigned this Jun 24, 2018
@anntzer anntzer removed their assignment Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants