-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
New Formatter API #5804
Conversation
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 Please revert the removal of |
Backwards compatibilityI 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 What API do you suggest for The format of the offset textNote 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): |
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.
Please do not use kwarg popping and please preserve the current API (useMathText
).
Formatting suggestion: put spaces around the |
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.
77774bc
to
bb7c536
Compare
Slowly working on restoring backcompat. I think mathtext already handles spacing around operators (or at least it should?). |
bb7c536
to
60ceae9
Compare
60ceae9
to
277574f
Compare
self._round_to = round_to | ||
|
||
def __call__(self, x, pos=None): | ||
def format_for_tick(self, x, pos=None): |
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.
Again a much needed improvement IMHO. If we really want __call__()
functionality I would just call format_for_tick()
without any additional code.
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.
Dropping __call__
is a pretty big API change.
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 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.
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.
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*
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 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.
I just had an actual use case where the reliance on 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
(instead of calling I guess backwards compatibility comes into play in two places here:
In a sense, it may even be a better improvement if |
I think you may be working around this the wrong way. Use |
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. |
T he You can use a 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. |
rather than the, IMO, simpler
|
@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. |
attn @NelleV |
Note to self, once we get to revisit this issue: we could also have nearly stateless formatters, which are simply callables with the signature I say nearly stateless because we want to maintain configurability ( |
Closing as bitrotted, but probably worth revisiting at some point. |
This is a complete rewrite of the Formatter API, as suggested first in #5787.
Axes.fmt_{x,y}data
has been removed as assignment toAxes.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.