-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Overload the function signature of axline #29235
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
base: main
Are you sure you want to change the base?
Overload the function signature of axline #29235
Conversation
…pecify xy2, and another having the option to specify slope
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 opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. 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.
…om/li-ruihao/matplotlib into axline_definition_overload
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 believe the | None
options should go on the overload as the parameters xy2 and slope are not optional for the specific cases.
I have not thought this through completely yet, but I do think that these are made slightly harder by a few factors. I think Tim is right about removing the I am, however, not sure that that will have the desired result, especially once it goes through One option would be to move this method out of the autogenerated portion of pyplot and just include the overloads (and subsequent implementation) in the handwritten portion. We do this for a few things already. This is perhaps a reasonable short term action (though I would have to look again at the precise actions that would need to be taken there). One thing that complicates the analysis at least it is the inclusion of |
I initially removed the However, in order to keep the original behaviors of the function without introducing any regression errors, I added the
Test cases 1 and 2 worked as expected and successfully drew a line, test cases 3 and 4 threw errors with a message that says only one of xy2 or slope must be presented, which were working as expected as well. Therefore confirming the current changes did not introduce any regression errors. It does make sense to remove the |
PR summary
Originally within lib/matplotlib/axes/_axes.pyi,
There was a TODO item:
The change in this PR is to overload the definition of axline with two separate definitions, with one needed to specify
xy2
and the other to specifyslope
.This PR closes issue #29234.
PR checklist