Repository navigation
BUG: WCSAxes.grid's signature is incompatible with parent class #15987
Description
Description
astropy.visualization.wcsaxes.core.WCSAxes is derived from matplotlib.axes.Axes.
With time, WCSAxes.grid's signature drifted apart from its parent class', leading to a LSP (Liskov Substitution Principle) violation.
Here's the current document for the parent method
https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.grid.html#matplotlib.axes.Axes.grid
And here's the child definition
astropy/astropy/visualization/wcsaxes/core.py
Line 771 in 3d5aae9
- the first argument
bwas renamedvisible(as a boolean, it's common style to pass it as keyword, which makes it a problem) whichis accepted as positional in the parent method but is keyword-only in the child methodaxisis accepted positionally in both, but is on third position in the parent method and on second position in the child method
The most recent change (b renamed to visible) happened in matplotlib v3.5.0, and we currently support matplotlib 3.3, so there's an additional layer of complexity if we want to match the parent definition in a version-dependent way.
Luckily, matplotlib 3.6 is currently the oldest version recommended by SPEC 0 so we could just bump the requirement.
The question is how to address these incompatibilities gradually (as opposed to just changing the signature in one go, which would be a breaking change)
Problem 1) can be solved using @deprecated_renamed_argument, however it's not the case of problems 2) and 3), which I'm not sure can be resolved over a deprecation period. I'm thinking we could just resolve problem 1) in a minor release, and 2) and 3) would need to be addressed in a major one.
A related issue is that astropy.visualization.wxsaxes.coordinate_helpers.CoordinateHelper.grid takes a draw_grid argument (which is equivalent to the visible argument) and arbitrary keyword arguments (**kwargs) that are passed from astropy.visualization.wcsaxes.core.WCSAxes.grid, in a way that passing visible as keyword in astropy.visualization.wcsaxes.core.WCSAxes.grid has no effect.
Originally discussed in #15985 (review)
Expected behavior
No response
How to Reproduce
No response
Versions
No response
Activity
pllim commentedon Feb 5, 2024
cc @astrofrog and @Cadair and maybe @dstansby
astrofrog commentedon Jun 19, 2025
Thanks for the report! I'm not sure that 2) is really a problem, is it? It's ok for us to enforce that
which=has to be a keyword argument, and no users will currently be using it as a positional argument. If we start to allow it as positional, it will be hard to go backwards in future if Matplotlib make it keyword-only?If we want to give ourselves flexibility in future if Matplotlib change the API again, we should probably just accept
*argsand**kwargsand have some logic that will interpret it in different ways depending on what is passed?