Fix floating point inaccuracies in axes limits; partial work on offset value selection #5768

anntzer · 2015-12-31T05:49:00Z

(1) plt.plot([-.1, .2]) used to pick (in round numbers mode) [-.1, .25] as
ylims due to floating point inaccuracies; change it to pick [-.1, .2]
(up to floating point inaccuracies).

Note that this requires working around a bug in numpy's implementation
of divmod (numpy/numpy#6127).

Many test images have changed!

See #5767.

The following part is incorrect and only left for reference purposes, please ignore.

(2) Make MaxNLocator only return points within bounds.

e.g. when the xlims are -0.5 .. 10.5, return 0 .. 10 instead of -1
.. 11.

This implies dropping the (unused and deprecated-in-comment)
"trim" keyword argument.

Also slightly clean up the implementation of scale_range.

Initial work on #5738.

mdboom · 2015-12-31T15:20:03Z

(2) Make MaxNLocator only return points within bounds. e.g. when the xlims are -0.5 .. 10.5, return 0 .. 10 instead of -1 .. 11.

Doesn't this imply not displaying all of the data? Or am I missing something?

mdboom · 2015-12-31T15:31:45Z

The twin_axes_spines_top example seems to be a case where data is being cut...

anntzer · 2016-01-01T01:46:48Z

Sorry, please ignore the second part of the original PR message (now edited). This was a left-over of when I thought the xlims were set independently of the ticks. I have also now melded the two commits together as they don't really make sense independently.

Regarding twin_spines_on_top, in fact no, there's no data being cut. The green line reaches 0.40 on the right, and the old image had a y axis going up to 0.45; now it only goes up to 0.40 (re-run the test without thickening the lines to see it).

efiring · 2016-01-01T01:59:23Z

lib/matplotlib/ticker.py

@@ -168,6 +169,15 @@
    long = int


+# Work around numpy/numpy#6127.
+def divmod(x, y):


This should be private (leading underscore), shouldn't it?

It's not listed in __all__, just like a bunch of other functions in the same module (closeto, decade_{up,down}, nearest_long, etc.) which are not prefixed by an underscore but still arguably private.

Based on your argument, we would never use underscores. There are two reasons for using one here:

With its present name, this function is shadowing a builtin.

It is a workaround for a bug. Some time after the bug is fixed, we will want to remove this function. If it has an underscore, we don't need to go through a deprecation process, or argue about whether to do so.

Also, I see that divmod is used in 3 other modules; is there reason to use this version in those locations as well? If so, then it could be put in cbook.

I am with @efiring, please add an underscore. There are parts of mpl that pre-date the 'underscore means private' convention in python.

A quick grep through all other calls to divmod in the codebase suggests that they are always called either with integer arguments (in which case there's no bug) or with (nontrivial) array arguments (in which case the fix I used is not enough). So I'll just leave it here (as _divmod) for now.

The ternary-if has also been fixed.

anntzer · 2016-01-01T03:03:32Z

I'm a bit confused by the Travis failure only for 2.7. Everything passes fine for me (https://travis-ci.org/anntzer/matplotlib/builds/99672606), and Appveyor also seems happy. Thoughts?

jenshnielsen · 2016-01-01T09:48:59Z

It's a random failure not related to your work see #5647 (this was the test__EventCollection__set_linestyle.test again)

I have restarted the test

efiring · 2016-01-01T16:05:24Z

Apart from my inline comments, this looks like a nice improvement. Thanks for taking it on.

tacaswell · 2016-01-02T02:00:17Z

lib/matplotlib/ticker.py


-    def bin_boundaries(self, vmin, vmax):


Do we need to go through a deprecation cycle for bin_boundaries?

Good question. Can we avoid that simply by renaming _raw_ticks back to bin_boundaries?

On the other hand, we have been saying for a while we want to reduce the size of the public API and this is part of the internal details of how the MaxN locator works so it should be private.

Note tha the behavior of _raw_ticks is slightly different from bin_boundaries (namely it handles properly floating-point inaccuracies), so given that strict backwards compatibility has been broken I think it's a good opportunity to make it private. If you prefer I can leave the (now unused) old implementation of bin_boundaries in, with a deprecation warning.

A bug fix (handling floating point better) would not in itself require a name change or deprecation, but I'm perfectly happy with the new private name. To be on the safe side, a deprecation cycle of the old name would be a good idea. (I thought it was being used elsewhere, but I haven't found any such instances.)

Restored bin_boundaries with a deprecation warning since v2.0.

mdboom · 2016-01-04T15:27:36Z

Regarding twin_spines_on_top, in fact no, there's no data being cut. The green line reaches 0.40 on the right, and the old image had a y axis going up to 0.45; now it only goes up to 0.40 (re-run the test without thickening the lines to see it).

Ok. Thanks for confirming.

mdboom · 2016-01-04T15:28:14Z

This is a real nice refinement of a detail that's been broken for a very long time. Thanks for taking this on.

tacaswell · 2016-02-15T20:01:34Z

❤️ pep8

======================================================================
FAIL: matplotlib.tests.test_coding_standards.test_pep8_conformance_installed_files
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_coding_standards.py", line 249, in test_pep8_conformance_installed_files
    expected_bad_files=expected_bad_files)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_coding_standards.py", line 143, in assert_pep8_conformance
    assert_equal(result.total_errors, 0, msg)
AssertionError: 1 != 0 : Found code syntax errors (and warnings):
/home/travis/build/matplotlib/matplotlib/lib/matplotlib/ticker.py:1405:80: E501 line too long (80 > 79 characters)
----------------------------------------------------------------------

plt.plot([-.1, .2]) used to pick (in round numbers mode) [-.1, .25] as ylims due to floating point inaccuracies; change it to pick [-.1, .2] (up to floating point inaccuracies). Support for the (unused and deprecated-in-comment) "trim" keyword argument has been dropped as the way ticks are picked has changed. Many test images have changed! Implementation notes: - A bug in numpy's implementation of divmod (numpy/numpy#6127) is worked around. - The implementation of scale_range has also been cleaned. See matplotlib#5767, matplotlib#5738.

anntzer · 2016-02-15T21:11:27Z

Fixed and rebased.

jenshnielsen · 2016-03-20T18:28:00Z

IMHO this is ready for merge (Needs a rebase which I did in https://github.com/jenshnielsen/matplotlib/tree/tighter-axes-limits) but otherwise looks good

It would be nice to fix issues with numpy 1.11 which this should do. So the question is should we backport this to 2.x or 1.5.x?

anntzer · 2016-03-20T18:33:22Z

Can you PR the rebase to my branch?

jenshnielsen · 2016-04-03T18:10:25Z

For the record this was merged via #6192 (which is just a rebase onto master of the time)

tacaswell added the status: needs review label Dec 31, 2015

anntzer force-pushed the tighter-axes-limits branch from d327e74 to 4fd8a4e Compare January 1, 2016 01:46

efiring reviewed Jan 1, 2016
View reviewed changes

anntzer force-pushed the tighter-axes-limits branch from 4fd8a4e to 846fde4 Compare January 1, 2016 21:05

tacaswell reviewed Jan 2, 2016
View reviewed changes

anntzer force-pushed the tighter-axes-limits branch from 846fde4 to dae0b87 Compare January 2, 2016 03:27

anntzer mentioned this pull request Jan 13, 2016

extra label for non-existent tick #5841

Closed

anntzer force-pushed the tighter-axes-limits branch from dae0b87 to e67c7fc Compare February 15, 2016 21:11

anntzer mentioned this pull request Mar 20, 2016

Issues with Numpy 1.11 release candidate #5950

Closed

tacaswell mentioned this pull request Mar 20, 2016

More precise choice of axes limits. #6192

Merged

tacaswell closed this Mar 20, 2016

tacaswell removed the status: needs review label Mar 20, 2016

anntzer deleted the tighter-axes-limits branch March 20, 2016 23:32

QuLogic added this to the 2.0 (style change major release) milestone Apr 3, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix floating point inaccuracies in axes limits; partial work on offset value selection #5768

Fix floating point inaccuracies in axes limits; partial work on offset value selection #5768

anntzer commented Dec 31, 2015

mdboom commented Dec 31, 2015

mdboom commented Dec 31, 2015

anntzer commented Jan 1, 2016

efiring Jan 1, 2016

anntzer Jan 1, 2016

efiring Jan 1, 2016

tacaswell Jan 1, 2016

anntzer Jan 1, 2016

anntzer commented Jan 1, 2016

jenshnielsen commented Jan 1, 2016

efiring commented Jan 1, 2016

tacaswell Jan 2, 2016

efiring Jan 2, 2016

tacaswell Jan 2, 2016

anntzer Jan 2, 2016

efiring Jan 2, 2016

anntzer Jan 2, 2016

mdboom commented Jan 4, 2016

mdboom commented Jan 4, 2016

tacaswell commented Feb 15, 2016

anntzer commented Feb 15, 2016

jenshnielsen commented Mar 20, 2016

anntzer commented Mar 20, 2016

jenshnielsen commented Apr 3, 2016

Fix floating point inaccuracies in axes limits; partial work on offset value selection #5768

Fix floating point inaccuracies in axes limits; partial work on offset value selection #5768

Conversation

anntzer commented Dec 31, 2015

mdboom commented Dec 31, 2015

mdboom commented Dec 31, 2015

anntzer commented Jan 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Jan 1, 2016

jenshnielsen commented Jan 1, 2016

efiring commented Jan 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdboom commented Jan 4, 2016

mdboom commented Jan 4, 2016

tacaswell commented Feb 15, 2016

anntzer commented Feb 15, 2016

jenshnielsen commented Mar 20, 2016

anntzer commented Mar 20, 2016

jenshnielsen commented Apr 3, 2016