Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Add test_axhline in test_datetime.py#27177
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| birth_date = np.array([datetime.datetime(2020, 4, 10), | ||
| datetime.datetime(2020, 5, 30), | ||
| datetime.datetime(2020, 10, 12), | ||
| datetime.datetime(2020, 11, 15)]) | ||
| year_start = datetime.datetime(2020, 1, 1) | ||
| year_end = datetime.datetime(2020, 12, 31) | ||
| age = [21, 53, 20, 24] | ||
| ax.set_xlabel('Birth Date') | ||
| ax.set_ylabel('Age') | ||
| ax.set_ylim(bottom=year_start, top=year_end) | ||
| ax.bar(x=age, height=birth_date) |
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 say to keep this even more minimal and not have the bar graph (I think setting the ylim is important here)
If anything, I'd just do aax.plot, but even that I think is not testing what we are looking to test here.
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.
That sounds good, thank you for the feedback!
I simplified the example and added |
Uh oh!
There was an error while loading.Please reload this page.
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.
Could you add subplots to test different combinations of numbers and datetimes on different axes? Similar to the test you added here:#27084
Sure thing. Can you elaborate on what you mean by different axes? I can test axhline on different plots, but I am unsure how to utilize it on different axes as it itself only plots on the horizontal. Below is an image of what I have come up with so far. Thank you for your feedback! |
scottshambaugh commentedOct 24, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@CozyFrog ah I mean having subplots where the X/Y data are different combinations of datetime vs numbers. Right now you have datetimes on the Y axis with numbers on the X axis, and adding the other permutations of that should exercise these codepaths more fully (I think). Your other PR did this well. |
@scottshambaugh I don't think that actually makes sense for this method, honestly... the x axis values are in |
I'll defer to you on that@ksunden, I'm admittedly not super familiar with how the code paths for the coordinate transforms work work. If it's drawn directly in axes coordinates they I think you're right, my thought was if the xrange was cast to the data coordinates before being drawn. |
Thank you for the discussion@scottshambaugh &@ksunden – how should I move forward with this PR? Does this require any modifications? |
Yes, the X values are drawn directly in axes coordinates, and in fact the method explicitly checks that those valuesdo not have units and prevents passing an alternate transform even. Thus I think we should be good to go here, though we do have a 2-reviewer policy for non-doc changes (And I was the first approver, soo). |
PR summary
I have added a datetime (no timedelta; this function does not support it, and I believe that is correct) smoke test for
Axes.axhlineinlib/matplotlib/test/test_datetime.py.This addresses the
Axes.axhlinetask from#26864.The image below is the plot generated from my example code.
PR checklist