FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ENH: offset parameter for MultipleLocator by rcomer · Pull Request #25542 · matplotlib/matplotlib · GitHub

Repository navigation

ENH: offset parameter for MultipleLocator - #25542

Merged
ksunden merged 1 commit into
matplotlib:mainfrom
rcomer:multiple-locator-offset
Jun 3, 2023
Merged

ksunden merged 1 commit into
matplotlib:mainfrom
rcomer:multiple-locator-offset

Conversation

rcomer commented Mar 24, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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()

### 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 commented Mar 30, 2023

Copy link
Copy Markdown
Member Author

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

rcomer marked this pull request as draft March 31, 2023 06:21
rcomer force-pushed the multiple-locator-offset branch from 8796a8c to f386f33 Compare March 31, 2023 18:05

rcomer commented Mar 31, 2023

Copy link
Copy Markdown
Member Author

@ksunden could you help me decipher this Mypy failure?

# 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')

rcomer Mar 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 force-pushed the multiple-locator-offset branch from a6782e7 to 8c720a9 Compare March 31, 2023 18:54
rcomer force-pushed the multiple-locator-offset branch from 8c720a9 to ad37187 Compare April 1, 2023 06:54
Comment thread ci/mypy-stubtest-allowlist.txt Outdated
rcomer marked this pull request as ready for review April 1, 2023 07:36

rcomer commented Apr 1, 2023

Copy link
Copy Markdown
Member Author

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 commented Apr 1, 2023

Copy link
Copy Markdown
Member

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 commented Apr 1, 2023

Copy link
Copy Markdown
Member

Yes this should definitely get what's new entry

rcomer force-pushed the multiple-locator-offset branch 2 times, most recently from 1b82ad2 to 9ae5834 Compare April 1, 2023 15:32

rcomer commented Apr 1, 2023

Copy link
Copy Markdown
Member Author

OK thanks. What's new now added.

Co-authored-by: Ruth Comer 10599679+rcomer@users.noreply.github.com
ksunden merged commit f7524f4 into matplotlib:main Jun 3, 2023
rcomer deleted the multiple-locator-offset branch June 3, 2023 07:07
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL