-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
set offset threshold to 4 #7104
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
Comments
Please put in exactly the code snippets that yield the two example images, and elaborate on what it is that looks wrong. I can't tell from what you have here so far. |
I don't see anything wrong, except in your manually-adjusted example. The Formatter is using an offset of 2000, so the tick labels are from 2000 - 10 to 2000 + 20. In many cases like this one doesn't actually want an offset. It can be turned off with |
As efiring stated, this is relatively standard behavior, albeit rather counterintuitive. I don't know the source code well enough without looking; could there be room for improvement in the default behavior of deciding when to turn on offsetting? |
I fully fully support changing the default 'cause that offset seems so incredibly arbitrary and like you said, way counterintuitive. I think it looks at base 10 for defaults somehow, since something like Also, @efiring how come calling |
On 2016/09/13 9:15 AM, hannah wrote:
Setting the view limits is independent of the Formatter; the offset |
@efiring I just realized that half my issue is that I didn't read the notation correctly. Why is the default to pitch it to scientific notation for 4 numbers? (What is the cut off?) |
On 2016/09/13 9:27 AM, hannah wrote:
A non-zero offset is used if it saves at least 2 significant digits in It looks like the offset is always being forced into scientific |
I'm 👍 on that. |
@anntzer was doing some work on this, but I do not remember if it got merged (sorry). There is also #6086 which is still waiting for a review (sorry again @VincentVandalon) |
The choice of the offset value (not its formatting) was changed by myself in #5785. The formatting of the offset value is addressed by #5804 (so that the offset displays enough significant digits to match the ticks themselves, and so that the cursor values are also displayed in a useful manner), but this PR also lumps in a large amount of breaking API changes to the formatter class (which I still think seriously need some API cleanup). TBH I have lost interest in making this backwards compatible, I think it's too difficult to be worth it and now simply disable any use of offsets from my matplotlibrc. (I can update my PR if we agree to break backcompat but will probably not add a complete backcompat layer.) |
No problem @tacaswell we are all doing this on a voluntary basis (I think). In #6086 i tried to implement the desired behavior with as few as possible changes/refactoring. When you reviewed the pull request, you (rightly so) commented that it was not clear from the code if the desired behavior was achieved. The code was not transparent to begin with, and it did not improve with my changes. I started refactoring to make the code clear, but that overlapped with #5804 and #5785. So #6068 ended up in limbo. If the ticker.py code has undergone refactoring in the mean time, we could close #6086 |
@story645 do you see a way out of the impasse here? If not, I think this needs to be booted down the road to v2.1. |
There's a relevant discussion of the formatting of the offset value in the first three messages of #5804, FWIW. |
Lets bump the threshold up to 3 or 4 digits to make years behave better. There are enough questions on SO specifically related to this issue with years (and I have seen it go wrong for my co workers) is worth fixing. |
Ah, I didn't think about that use case. Seems reasonable to me. |
@efiring I'm for whatever solution is easy and more user friendly. I support @tacaswell's suggestion of making a hard minimum of 4 digits since years are very common use case, so much so that it's the usecase that triggered my opening this issue. |
https://github.com/matplotlib/matplotlib/blob/v2.x/lib/matplotlib/ticker.py#L693 is the expression that needs to change, someone just needs to understand it enough to make the threshold 4 instead of 2.... |
In the line below that, I would try changing to this: if abs_max // 10 ** oom >= 1000 |
Fixed by #7365. |
Using the code from #7084,

ax.set_xlim(1990, 2020)
yieldsTried this for some other values and it seems broken for everything except values that follow the form %d*10^n so 10, 20, 300, etc.
Eta this also yields weirdness:

ax.plot(range(1990,2020), range(1990,2020))
The text was updated successfully, but these errors were encountered: