Skip to content

Deprecate 'prune' kwarg to MaxNLocator#25191

Closed
dstansby wants to merge 1 commit intomatplotlib:mainfrom
dstansby:deprecate-prune
Closed

Deprecate 'prune' kwarg to MaxNLocator#25191
dstansby wants to merge 1 commit intomatplotlib:mainfrom
dstansby:deprecate-prune

Conversation

@dstansby
Copy link
Copy Markdown
Member

PR Summary

We do not seem to use prune in any tests or examples, so it seems resonable to deprecate because:

  • There's not loads of code to support this, but it's still a little bit of bloat on the already complex MaxNLocator
  • It's easy enough for users to replicate the behaviour by simple indexing after the tick values are returned
  • prune is not particularly new, dating back 14 years: 0c79ba3

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

@dstansby dstansby added this to the v3.8.0 milestone Feb 10, 2023
@dstansby dstansby marked this pull request as ready for review February 10, 2023 20:15
Comment on lines +2052 to +2054
"3.8", message="The 'prune' keyword argument is deprecated, "
"and will be removed in Matplotlib %(removal)s. Tick pruning "
"should be done manually after the ticks have been generated."
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The last sentence should be passed to addendum instead of overriding message.

integer=False,
symmetric=False,
prune=None):
# trim argument has no effect. It has been left for API compatibility
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should keep this comment, or also look into deprecating it as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make steps keyword only?

@@ -0,0 +1,6 @@
Automatically pruning ticks in ``MaxNLocator``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The ``prune`` keyword argument to both `matplotlib.ticker.MaxNLocator` and
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
The ``prune`` keyword argument to both `matplotlib.ticker.MaxNLocator` and
The *prune* keyword argument to both `matplotlib.ticker.MaxNLocator` and

see `.MaxNLocator`
symmetric : bool, optional
see `.MaxNLocator`
prune : {'lower', 'upper', 'both', None}, optional
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not remove yet? Add deprecation notice instead?

@dstansby
Copy link
Copy Markdown
Member Author

It's been too long since I looked at this, and the rebase isn't simple, so I'm going to close. If someone really thinks we should get this in please shout, otherwise I won't pursue it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants