Skip to content

MNT: simplify valid tick logic #12158

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

Merged
merged 1 commit into from
Jan 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 14 additions & 47 deletions lib/matplotlib/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1028,62 +1028,29 @@ def _update_ticks(self, renderer):
ihigh = locs[-1]
tick_tups = [ti for ti in tick_tups if ilow <= ti[1] <= ihigh]

# so that we don't lose ticks on the end, expand out the interval ever
# so slightly. The "ever so slightly" is defined to be the width of a
# half of a pixel. We don't want to draw a tick that even one pixel
# outside of the defined axis interval.
if interval[0] <= interval[1]:
interval_expanded = interval
else:
interval_expanded = interval[1], interval[0]

if hasattr(self, '_get_pixel_distance_along_axis'):
# normally, one does not want to catch all exceptions that
# could possibly happen, but it is not clear exactly what
# exceptions might arise from a user's projection (their
# rendition of the Axis object). So, we catch all, with
# the idea that one would rather potentially lose a tick
# from one side of the axis or another, rather than see a
# stack trace.
# We also catch users warnings here. These are the result of
# invalid numpy calculations that may be the result of out of
# bounds on axis with finite allowed intervals such as geo
# projections i.e. Mollweide.
with np.errstate(invalid='ignore'):
try:
ds1 = self._get_pixel_distance_along_axis(
interval_expanded[0], -0.5)
except Exception:
cbook._warn_external("Unable to find pixel distance "
"along axis for interval padding of "
"ticks; assuming no interval "
"padding needed.")
ds1 = 0.0
if np.isnan(ds1):
ds1 = 0.0
try:
ds2 = self._get_pixel_distance_along_axis(
interval_expanded[1], +0.5)
except Exception:
cbook._warn_external("Unable to find pixel distance "
"along axis for interval padding of "
"ticks; assuming no interval "
"padding needed.")
ds2 = 0.0
if np.isnan(ds2):
ds2 = 0.0
interval_expanded = (interval_expanded[0] - ds1,
interval_expanded[1] + ds2)
if interval[1] <= interval[0]:
interval = interval[1], interval[0]
inter = self.get_transform().transform(interval)

ticks_to_draw = []
for tick, loc, label in tick_tups:
# draw each tick if it is in interval. Note the transform
# to pixel space to take care of log transforms etc.
# interval_contains has a floating point tolerance.
if tick is None:
continue
# NB: always update labels and position to avoid issues like #9397
tick.update_position(loc)
tick.set_label1(label)
tick.set_label2(label)
if not mtransforms.interval_contains(interval_expanded, loc):
try:
loct = self.get_transform().transform(loc)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it really makes a performance difference, but AFAICS transform() works on numpy arrays. So this could be pulled out of the loop and calculated in one go. Well, except for the AssertionError - when does that occur?

Copy link
Member Author

Choose a reason for hiding this comment

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

We still have to do the loop, so can probably keep as-is for now.

I think the Assertion Error happened in CI, possibly get_transform returned None for some wacky axis.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah its from the doc build: gallery/scales/custom_scale.py.

Exception occurred:
  File "/home/circleci/project/lib/matplotlib/transforms.py", line 1436, in transform
    assert not np.ma.is_masked(res)  # just to be on the safe side
AssertionError

The custom scale uses masks, and transforms.transform doesn't like it.

The previous implementation didn't call the transform, but this one does so that we can do the tolerance checking in log-scale rather than data space.

Copy link
Member

Choose a reason for hiding this comment

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

Are we sure that there are still any ticks on that example when we hit this?

except AssertionError:
# transforms.transform doesn't allow masked values but
# some scales might make them, so we need this try/except.
loct = None
continue
if not mtransforms._interval_contains_close(inter, loct):
continue
ticks_to_draw.append(tick)

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified lib/matplotlib/tests/baseline_images/test_axes/specgram_noise.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
33 changes: 31 additions & 2 deletions lib/matplotlib/transforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -2900,10 +2900,39 @@ def interval_contains(interval, val):
Returns
-------
bool
Returns true if given val is within the interval.
Returns *True* if given *val* is within the *interval*.
"""
a, b = interval
if a > b:
a, b = b, a
return a <= val <= b


def _interval_contains_close(interval, val, rtol=1e-10):
"""
Check, inclusively, whether an interval includes a given value, with the
interval expanded by a small tolerance to admit floating point errors.

Parameters
----------
interval : sequence of scalar
A 2-length sequence, endpoints that define the interval.
val : scalar
Value to check is within interval.
rtol : scalar
Tolerance slippage allowed outside of this interval. Default
1e-10 * (b - a).

Returns
-------
bool
Returns *True* if given *val* is within the *interval* (with tolerance)
"""
a, b = interval
return a <= val <= b or a >= val >= b
if a > b:
a, b = b, a
rtol = (b - a) * rtol
return a - rtol <= val <= b + rtol


def interval_contains_open(interval, val):
Expand Down