Code review comments

Author: scottshambaugh

lib/matplotlib/axis.py

@@ -871,6 +871,15 @@ def limit_range_for_scale(self, vmin, vmax):
         """
         return self._scale.limit_range_for_scale(vmin, vmax, self.get_minpos())
 
+    def _nan_out_of_scale_range(self, data):
+        """
+        Return *data* with values that are out of range for this axis's scale
+        replaced by NaN. E.g. ``<=0`` on a log axis.
+        """
+        data = np.asanyarray(data, dtype=float)
+        valid = self._scale.val_in_range(data)
+        return data if np.all(valid) else np.where(valid, data, np.nan)
+
     def _get_autoscale_on(self):
         """Return whether this Axis is autoscaled."""
         return self._autoscale_on

lib/matplotlib/tests/test_scale.py

@@ -481,22 +481,18 @@ def test_val_in_range_base_fallback():
 
 def test_val_in_range_array():
     # Vectorized: scalar in -> scalar bool, array in -> bool array.
-    arr = np.array([1.0, -1.0, 0.0, np.nan, np.inf, 5.0])
+    arr = np.array([0.5, -1.0, 0.0, np.nan, np.inf, 0.25])
     cases = {
         'linear': [True, True, True, False, False, True],
         'log':    [True, False, False, False, False, True],
         'symlog': [True, True, True, False, False, True],
         'asinh':  [True, True, True, False, False, True],
+        'logit':  [True, False, False, False, False, True],
     }
     for name, expected in cases.items():
         s = mscale._scale_mapping[name](axis=None)
         np.testing.assert_array_equal(s.val_in_range(arr), expected)
 
-    s = mscale._scale_mapping['logit'](axis=None)
-    np.testing.assert_array_equal(
-        s.val_in_range(np.array([0.1, 0.5, 0.0, 1.0, -0.1, 1.1])),
-        [True, True, False, False, False, False])
-
     # 2D shape is preserved.
     out = mscale._scale_mapping['log'](axis=None).val_in_range(
         np.array([[1.0, -1.0], [0.5, np.nan]]))

lib/mpl_toolkits/mplot3d/art3d.py

@@ -217,7 +217,7 @@ def set_3d_properties(self, z=0, zdir='z', axlim_clip=False):
     def draw(self, renderer):
         mask = _scale_invalid_mask(self._x, self._y, self._z, self.axes)
         if self._axlim_clip:
-            mask = mask | _viewlim_mask(self._x, self._y, self._z, self.axes)
+            mask |= _viewlim_mask(self._x, self._y, self._z, self.axes)
         if np.any(mask):
             pos3d = np.ma.array([self._x, self._y, self._z],
                                 mask=mask, dtype=float).filled(np.nan)
@@ -356,7 +356,7 @@ def get_data_3d(self):
     def draw(self, renderer):
         scale_mask = _scale_invalid_mask(*self._verts3d, self.axes)
         if self._axlim_clip:
-            scale_mask = scale_mask | _viewlim_mask(*self._verts3d, self.axes)
+            scale_mask |= _viewlim_mask(*self._verts3d, self.axes)
         if np.any(scale_mask):
             mask = np.broadcast_to(
                 scale_mask,

lib/mpl_toolkits/mplot3d/axes3d.py

@@ -58,20 +58,6 @@
 from . import axis3d
 
 
-def _mask_scale_invalid(data, axis):
-    """
-    Return ``data`` with values invalid for ``axis``'s scale (e.g. ``<=0`` on a
-    log axis) replaced by NaN, so they don't pollute data limits.
-    """
-    if data is None:
-        return data
-    data = np.asanyarray(data, dtype=float)
-    valid = axis._scale.val_in_range(data)
-    if np.all(valid):
-        return data
-    return np.where(valid, data, np.nan)
-
-
 @_docstring.interpd
 @_api.define_aliases({
     "xlim": ["xlim3d"], "ylim": ["ylim3d"], "zlim": ["zlim3d"]})
@@ -654,16 +640,16 @@ def autoscale(self, enable=True, axis='both', tight=None):
     def auto_scale_xyz(self, X, Y, Z=None, had_data=None):
         # This updates the bounding boxes as to keep a record as to what the
         # minimum sized rectangular volume holds the data.
-        X = _mask_scale_invalid(X, self.xaxis)
-        Y = _mask_scale_invalid(Y, self.yaxis)
+        X = self.xaxis._nan_out_of_scale_range(X)
+        Y = self.yaxis._nan_out_of_scale_range(Y)
         if np.shape(X) == np.shape(Y):
             self.xy_dataLim.update_from_data_xy(
                 np.column_stack([np.ravel(X), np.ravel(Y)]), not had_data)
         else:
             self.xy_dataLim.update_from_data_x(X, not had_data)
             self.xy_dataLim.update_from_data_y(Y, not had_data)
         if Z is not None:
-            Z = _mask_scale_invalid(Z, self.zaxis)
+            Z = self.zaxis._nan_out_of_scale_range(Z)
             self.zz_dataLim.update_from_data_x(Z, not had_data)
         # Let autoscale_view figure out how to use this data.
         self.autoscale_view()