-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Remove datetime test for axes.pie #27117
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
Remove datetime test for axes.pie #27117
Conversation
Both u7277320 and sheepfan0828 are my personal Github account |
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.
Is it okey that some checks are not passed? |
The linting would need to be addressed. However, more fundamentally, this is not exercising the datetime unit machinery that is expected here, and in fact As such I think The list from the original issue/stubs was intentionally a bit broad but part of the idea was to evaluate whether it was using units at all. In this case it is not, and so it should just be removed. |
@ksunden Sorry, what do you mean by that? I cannot understand, can you please explain it more specific? |
This method falls into the category that passing datetimes to it directly does not make sense and therefore should just be removed (From #26864):
By calling |
So do you mean that is the method that i am testing with ( |
You can update this PR, no need to make a new one, but yes, I am saying that |
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 would suggest squash merging this (unless @Sheepfan0828 you wish to squash commits see here)
I took the liberty of updating the PR title as we are no longer smoke testing this method. Thanks @Sheepfan0828 and congratulations on your first Matplotlib PR! We hope to hear from you again. |
PR summary
The task i am working with is

Axe.pie
problem from the issue #26864 . TheAxe.pie
should be removed from the filetest_datetime.py
file because we cannot directly pass thedatetime
object intopie
as a parameter to perform any of the plots. The error message below shows the case occured whendatetime
object is directly passed into thepie
function:PR checklist