Unify set_pickradius argument #23196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

greglucas merged 1 commit into matplotlib:main from oscargus:pickradiusunification

Aug 4, 2022

Merged

Unify set_pickradius argument #23196

greglucas merged 1 commit into matplotlib:main from oscargus:pickradiusunification

Aug 4, 2022

Conversation

oscargus

Copy link

Member

@oscargus oscargus commented Jun 3, 2022

PR Summary

Always store in self._pickradius. Added property where needed. Added check for positive number (not for Collection since a negative number can be used according to docs).

Not sure if the rename_parameter is really needed since these are typically not called using kwargs, but better safe than sorry.

PR Checklist

Tests and Styling

[N/A] Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

[N/A] New features are documented, with examples if plot related.
[N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

@oscargus oscargus added API: consistency Maintenance labels

Jun 3, 2022

greglucas

greglucas approved these changes

Jun 5, 2022

View reviewed changes

@greglucas

Copy link

Contributor

greglucas commented Jun 5, 2022

/home/circleci/project/lib/matplotlib/axis.py:docstring of matplotlib.axis.Axis:45: WARNING: py:obj reference target not found: pickradius

@oscargus

Copy link

Member Author

oscargus commented Jun 5, 2022 •

edited

Loading

I do not really understand the error. It seems like it picks up the docstring from get_pickradius method now, as opposed to earlier where it got the docstring from the constructor argument(?).

I made it a property rather than directly accessing the attribute (although there were still get_pickradius and set_pickradius). I could of course revert that part, but it seems somehow more correct to use a private attribute and a property (simply skipping the property could break stuff elsewhere I assume).