8000 ENH: offset parameter for MultipleLocator by rcomer · Pull Request #25542 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

ENH: offset parameter for MultipleLocator #25542

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.

Sign up for GitHub

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
merged 1 commit into from
Jun 3, 2023
Merged

ENH: offset parameter for MultipleLocator #25542

merged 1 commit into from
Jun 3, 2023

Conversation

rcomer
Copy link
Member
@rcomer rcomer commented Mar 24, 2023 •
edited
Loading

PR Summary

Picked up from #21168. Adds an offset parameter to MultipleLocator so the ticks can all be shifted by a given value.

import matplotlib.pyplot as plt
import matplotlib.ticker as mticker

_, ax = plt.subplots()
ax.plot(range(10))
locator = mticker.MultipleLocator(base=3, offset=0.3)
ax.xaxis.set_major_locator(locator)

plt.show()

test

### To Do

1. Agree whether offset is the best name for the new parameter.
2. Address documentation tasks in the below checklist, but I wanted to get feedback on the principle and the parameter name before I went all in.

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/
  • [N/A] 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

@rcomer rcomer added this to the v3.8.0 milestone Mar 24, 2023
@rcomer rcomer mentioned this pull request Mar 24, 2023
Closed
7 tasks
@rcomer rcomer force-pushed the multiple-locator-offset branch from 0d93384 to 8796a8c Compare March 30, 2023 20:08
@rcomer
Copy link
Member Author
rcomer commented Mar 30, 2023

I pushed a change without rebasing just to see what Mypy would do. It's running....

@QuLogic QuLogic mentioned this pull request Mar 31, 2023
Merged
@rcomer rcomer marked this pull request as draft March 31, 2023 06:21
@rcomer rcomer force-pushed the multiple-locator-offset branch from 8796a8c to f386f33 Compare March 31, 2023 18:05
@rcomer
Copy link
Member Author
rcomer commented Mar 31, 2023

@ksunden could you help me decipher this Mypy failure?

rcomer
rcomer commented Mar 31, 2023
View reviewed changes
galleries/examples/ticks/tick-locators.py
@@ -53,7 +53,7 @@ def setup(ax, title):

# Index Locator
setup(axs[4], title="IndexLocator(base=0.5, offset=0.25)")
axs[4].plot(range(0, 5), [0]*5, color='white')
axs[4].plot([0]*5, color='white')
Copy link
Member Author
@rcomer rcomer Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's not completely obvious from looking at this example what the difference is between MultipleLocator and IndexLocator. But from the API docs it seems to me that IndexLocator makes most sense when the axis does not have numbers associated with it. So I took out the x-values.

I clearly should have done more homework before the Thursday call, then I would have known that offset is already used as a locator param 🤦‍♀️

@rcomer rcomer force-pushed the multiple-locator-offset branch from a6782e7 to 8c720a9 Compare March 31, 2023 18:54
@rcomer rcomer force-pushed the multiple-locator-offset branch from 8c720a9 to ad37187 Compare April 1, 2023 06:54
rcomer
rcomer commented Apr 1, 2023
View reviewed changes
@rcomer rcomer marked this pull request as ready for review April 1, 2023 07:36
@rcomer
Copy link
Member Author
rcomer commented Apr 1, 2023

I think this is now ready for review. I have not [yet] added a what's new entry, as I'm still pretty unclear in general what the criteria are for deciding to do so.

@oscargus
Copy link
Member
oscargus commented Apr 1, 2023

I'm still pretty unclear in general what the criteria are for deciding to do so.

.. versionadded:: 3.8 I think (or just that it is nice to showcase some new features!).

(I happened to read https://matplotlib.org/stable/devel/coding_guide.html#new-features-and-api-changes the other day...)

Parameter makes sense! (Will review code when I have a bit more time.)

@jklymak
Copy link
Member
jklymak commented Apr 1, 2023

Yes this should definitely get what's new entry

@rcomer rcomer force-pushed the multiple-locator-offset branch 2 times, most recently from 1b82ad2 to 9ae5834 Compare April 1, 2023 15:32
@rcomer
Copy link
Member Author
rcomer commented Apr 1, 2023

OK thanks. What's new now added.

@jondoesntgit @rcomer
Added option for an offset for MultipleLocator
1ad052e 
Co-authored-by: Ruth Comer 10599679+rcomer@users.noreply.github.com
@rcomer rcomer force-pushed the multiple-locator-offset branch from 9ae5834 to 1ad052e Compare May 25, 2023 06:45
@ksunden ksunden merged commit f7524f4 into matplotlib:main Jun 3, 2023
@rcomer rcomer deleted the multiple-locator-offset branch June 3, 2023 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@ksunden ksunden ksunden approved these changes

Assignees
No one assigned
Labels
New feature topic: ticks axis labels
Projects
None yet
Milestone
v3.8.0
Development

Successfully merging this pull request may close these issues.
6 participants
@rcomer @oscargus @jklymak @QuLogic @ksunden @jondoesntgit
0