ENH: Add which parameter to Axis.get_gridlines#31674
Open
SharadhNaidu wants to merge 2 commits into
Open
Conversation
`Axis.get_gridlines` previously returned only major gridlines, with no public way to get the minor gridlines. Downstream libraries (seaborn, gwpy, mpld3, ...) had to reach into the private `_minor_tick_kw` mapping to inspect minor grid state. Extend the signature to `get_gridlines(which='major')`, accepting `'major'`, `'minor'`, or `'both'` (matching the vocabulary of `Axis.grid` and `get_ticklabels`). The default of `'major'` keeps the existing behavior byte-for-byte. Closes matplotlib#19021
55bc9a1 to
676d602
Compare
The Axes.get_xgridlines and Axes.get_ygridlines methods are auto-generated wrappers around Axis.get_gridlines (via _axis_method_wrapper), so they inherit the new which parameter at runtime. Update the .pyi stubs to match, fixing the mypy stubtest failure.
Member
|
Please fill out the AI disclosure. |
This comment was marked as outdated.
This comment was marked as outdated.
Author
|
@rcomer Sorry for disturbing but , are there any changes that i have to make with the PR or is the review still pending ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR summary
Closes #19021.
Axis.get_gridlines()currently returns only major gridlines, with no public way to obtain the minor gridlines. As called out in the issue thread, downstream libraries have been forced to reach into private API as a workaround:seabornreadsAxis._minor_tick_kw['gridOn'](source)gwpy,mpld3,mplexporterand others were referenced in the same threadThis PR implements the API @jklymak proposed in the issue:
The vocabulary
'major' | 'minor' | 'both'matchesAxis.grid()andAxis.get_ticklabels(). Default'major'preserves the previous behavior byte-for-byte, so every existing zero-argument caller continues to work unchanged.Files changed
lib/matplotlib/axis.pyget_gridlinesto acceptwhich='major'/'minor'/'both'lib/matplotlib/axis.pyilib/matplotlib/axes/_base.pyiAxes.get_x/ygridlinesstubs (auto-delegated wrappers)lib/matplotlib/tests/test_axes.pydoc/release/next_whats_new/get_gridlines_which.rstTesting
I monkey-patched the new implementation onto an installed
matplotlib==3.10.9and ran the same assertions the new pytest cases exercise:which='major',which='minor',which='both'returns the expected lengths and ordering'both'ordering ismajor + minor(matchingget_ticklabels)Line2Dobjects tracks the active grid state throughAxis.grid(visible=..., which=...)callsget_gridlines()andget_gridlines('major')are equivalent on bothxaxisandyaxiswhich='invalid'raisesValueErrorwith the standard_api.check_in_listmessageAxes.get_xgridlines(which=...)/Axes.get_ygridlines(which=...)auto-delegate via_axis_method_wrapperAll checks pass against the installed matplotlib with the new method patched in.
Notes for reviewers
get_majorgridlines/get_minorgridlinesmethods (briefly considered in the thread) I am happy to refactor.which='both', major gridlines come before minor gridlines, matchingget_ticklabels(which='both').Line2D.get_visible()to address @anntzer's concern about per-gridline visibility.AI Disclosure
AI was used to help proofread the grammar of this PR description and to understand some parts of the existing codebase during development. The code changes and design decisions were developed independently.
PR checklist
.. plot::directive showing minor-gridline coloring)versionchanged:: 3.11in the docstring +doc/release/next_whats_new/get_gridlines_which.rst)