-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Added get_xmargin(), get_ymargin() and get_zmargin() and tests. #26293
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
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work on this @turnipseason. It looks good and I just have some m 8000 inor 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
8000 | """ | |
Retrieve autoscaling margin of the x-axis. | ||
|
||
.. versionadded:: 3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. 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! |
PR summary
Closes #26281 by adding
get_xmargin()
,get_ymargin()
for_AxesBase
andget_zmargin()
forAxes3D
, as well as tests for them.PR checklist