Deprecate nth_coord parameter from FixedAxisArtistHelper.new_fixed_axis. #27095
Conversation
anntzer
force-pushed
the
unc
branch
2 times, most recently
from
88fc605
to
55a182d
Compare
| def __init__(self, axes, loc, nth_coord=None): | ||
| """ | ||
| nth_coord = along which coordinate value varies | ||
| in 2D, nth_coord = 0 -> x axis, nth_coord = 1 -> y axis | ||
| """ | ||
| super().__init__(loc, nth_coord) | ||
| super().__init__(loc) |
There was a problem hiding this comment.
Doesn’t this make the parameter unused, which means we silently accept anything?
There was a problem hiding this comment.
But passing any value for nth_coord will now raise a DeprecationWarning anyways. (If you really want, I could duplicate the check here that nth_coord, if passed, is consistent with loc, but I think that's overkill. In any case I cannot forward it to the super class as that would also raise a DeprecationWarning.)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Passing an inconsistent *nth_coord* and *loc* (e.g., requesting a "top y axis" | ||
| or a "left x axis") to ``_FixedAxisArtistHelperBase`` or its subclasses is now | ||
| an error. |
There was a problem hiding this comment.
Given that nth_coord is now ignored, is it true that this is an error?
There was a problem hiding this comment.
fair enough, reworded.
| @@ -116,17 +116,11 @@ class _FixedAxisArtistHelperBase(_AxisArtistHelperBase): | |||
| lambda self: {"left": (0, 0), "right": (1, 0), | |||
| "bottom": (0, 0), "top": (0, 1)}[self._loc])) | |||
|
|
|||
| @_api.delete_parameter("3.9", "nth_coord") | |||
There was a problem hiding this comment.
Maybe add an addendum that this is inferred from loc?
| @@ -270,6 +271,7 @@ def update_grid_finder(self, aux_trans=None, **kwargs): | |||
| self.grid_finder.update(**kwargs) | |||
| self._old_limits = None # Force revalidation. | |||
|
|
|||
| @_api.delete_parameter("3.9", "nth_coord") | |||
There was a problem hiding this comment.
Should we not pass nth_coord_ticks to FixedAxisArtistHelper below? Then it will just be set to what was inferred by the parent class
matplotlib/lib/mpl_toolkits/axisartist/grid_helper_curvelinear.py
Lines 59 to 60 in 4846a8b
I think if we allow it to be passed down then we can still end up with something that is inconsistent with loc.
There was a problem hiding this comment.
Ah, good catch, the deprecation in the curvilinear case was too much, as for curvilinear axes (not floating axes) it does actually make sense to say, draw the first coordinate ticks on the vertical (second) axis, see e.g. the second example of https://matplotlib.org/stable/gallery/axisartist/demo_curvelinear_grid.html. Adjusted everything accordingly.
FixedAxisArtistHelpers place an axis on either the bottom, top, left, or right of an axis per (`loc`); we already infer whether it's an x-axis or a y-axis (`nth_coord`) from that and error out if there's a mismatch, so we may just as well delete the nth_coord parameter (which was already optional to start with). While at it, also expire deprecation for passing inconsistent nth_coord and loc.
FixedAxisArtistHelpers place an axis on either the bottom, top, left, or right of an axis per (
loc); we already infer whether it's an x-axis or a y-axis (nth_coord) from that and error out if there's a mismatch, so we may just as well delete the nth_coord parameter (which was already optional to start with).While at it, also expire deprecation for passing inconsistent nth_coord and loc.
PR summary
PR checklist