Plots first and last minor ticks #22331 #24661
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
eafe72c to
6351b6a
Compare
|
We're failing test_colorbar.py::test_colorbar_minorticks_on_off, and below is the image produced before our changes. |
6351b6a to
229d116
Compare
|
It looks like the failing test is failing because you've improved the output! The test sets the colorbar limits to -1.2, 1.2, but before your PR the ticks went from -1.1 to 1.3. With your PR, the ticks go from -1.2 to 1.2, which is better. So I think all that needs doing here is for you to update the list of expected ticks on line 399 in |
229d116 to
5fa31c4
Compare
There was a problem hiding this comment.
Please rebase so that you get the latest CircleCI config.
| (1, 0) # a single major tick => no minor tick | ||
| ] | ||
|
|
||
| def test_first_and_last_minorticks(self): |
There was a problem hiding this comment.
I'm a little concerned that this didn't test before!
| tmin = ((vmin - t0) // minorstep + 1) * minorstep | ||
| tmax = ((vmax - t0) // minorstep + 1) * minorstep | ||
| locs = np.arange(tmin, tmax, minorstep) + t0 | ||
| tmin = round((vmin - t0) / minorstep) |
There was a problem hiding this comment.
I think this needs an API change note. /doc/api/next_api_changes/behavior is the correct place for this.
|
Hi @aalmasmari are you still interested in working on this? I haven’t looked closely, but I think it just needs a changenote. |
Co-authored-by: Ali Almasmari <alialmas@umich.edu>
e3e3960 to
d1a9825
Compare
|
I think it would be good to get this in, so I took the liberty of squashing the commits and adding a changenote. I also reverted some white space changes. |
|
Thanks @aalmasmari! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again. |
PR Summary
Resolves issue where first and last minor ticks do not appear, as mentioned in issue #22331
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst