Added get_xmargin(), get_ymargin() and get_zmargin() and tests.#26293
Added get_xmargin(), get_ymargin() and get_zmargin() and tests.#26293story645 merged 11 commits intomatplotlib:mainfrom
Conversation
|
Hi @turnipseason is this one waiting for review? If so, please hit the "Ready for review" button. PRs marked as draft tend to be skipped over. |
rcomer
left a comment
rcomer
left a comment
There was a problem hiding this comment.
Thank you for your work on this @turnipseason. It looks good and I just have some minor comments.
|
Thanks for the update @turnipseason - looks good. I just learned (on gitter) that the new methods also need adding here so that they appear in the documentation. For most classes this is automatic and the methods appear in alphabetical order. For Axes we add them manually so we can group similar things together. |
There was a problem hiding this comment.
Might be overkill to add the version added directives here, but these should be mentioned in a whats new https://matplotlib.org/devdocs/devel/coding_guide.html#new-features-and-api-changes
Co-authored-by: hannah <story645@gmail.com>
| Getters for xmargin, ymargin and zmargin | ||
| ------------------------------------------------------------------ |
There was a problem hiding this comment.
| Getters for xmargin, ymargin and zmargin | |
| ------------------------------------------------------------------ | |
| Getters for xmargin, ymargin and zmargin | |
| ------------------------------------------- |
apparently I messed up and made it too long 🤦♀️ -> needs to be exact at edge
There was a problem hiding this comment.
Ah, sorry. I haven't written these before so I didn't know! I'll commit this version, then?
There was a problem hiding this comment.
By eye, that line still looks too long to me. Basically the dashes need to be the same length as the title.
There was a problem hiding this comment.
I think I should've done the previous commit by myself instead of via the github UI because it created a merge conflict. Not sure if what I did to solve it is right, but I think it's resolved in the latest commit, with the dashes being at the same level.
lib/matplotlib/axes/_base.py
Outdated
| """ | ||
| Retrieve autoscaling margin of the x-axis. | ||
|
|
||
| .. versionadded:: 3.7 |
There was a problem hiding this comment.
This would be in 3.9, as 3.7 is already out:
| .. versionadded:: 3.7 | |
| .. versionadded:: 3.9 |
lib/matplotlib/axes/_base.py
Outdated
| """ | ||
| Retrieve autoscaling margin of the y-axis. | ||
|
|
||
| .. versionadded:: 3.7 |
There was a problem hiding this comment.
| .. versionadded:: 3.7 | |
| .. versionadded:: 3.9 |
lib/mpl_toolkits/mplot3d/axes3d.py
Outdated
| """ | ||
| Retrieve autoscaling margin of the z-axis. | ||
|
|
||
| .. versionadded:: 3.7 |
There was a problem hiding this comment.
| .. versionadded:: 3.7 | |
| .. versionadded:: 3.9 |
|
Thank you so much for your persistence, this looks good to go. Would you like one of us to squash merge or would you like to try rebasing down to 1 commit? Either choice is fine, and we can help if you'd like to do the latter. |
Glad I could help! I'm a little bit afraid to screw things up with git😅. So I think it's better if you squash merge it. I'll read more on interactive rebasing (that's what would be used, right?) and try it some other time. |
Or regular rebasing, yup! |
5 participants
PR summary
Closes #26281 by adding
get_xmargin(),get_ymargin()for_AxesBaseandget_zmargin()forAxes3D, as well as tests for them.PR checklist