Skip to content
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

Add datetime testing skeleton #26859

Merged
merged 3 commits into from
Sep 22, 2023
Merged

Add datetime testing skeleton #26859

merged 3 commits into from
Sep 22, 2023

Conversation

tacaswell
Copy link
Member

@tacaswell tacaswell commented Sep 21, 2023

This is prep work for the 2023 GHC OSD event.

Although we nominally support units through all of our plotting APIs, this is not actually tested. As the most common "unitful" data is likely datetime, this is using datetime as the vehicle to exhaustively test which of the methods correctly handle units.

The ask of the volunteers will be to pick a method and write/find an example that passes datetime64 or timedelta64 arrays to as many of the inputs as make sense. Then

  1. if this works, open a PR adding the implementation as a test to this file + the output of the example when run as an image
  2. if this fails decide if:
    1. it should not work (for example ax.pie makes no sense with dates as the input so I pre-emptively removed it...although maybe with timedelta?) open a PR removing that stub test
    2. if it should work open an issue with the failing example

Unfortunately I did not get the test image overhaul done in time so I want to avoid adding test images for now, but smoke tests are still an improvement and once that lands it should be possible to "just" add a decorator to the tests to generate the images in the future.

@tacaswell tacaswell added this to the v3.9.0 milestone Sep 21, 2023
@story645
Copy link
Member

ax.pie makes no sense with dates as the input so I pre-emptively removed it...although

timedeltas though totally - thinking particularly of the American time use survey

@tacaswell
Copy link
Member Author

put pie back

@tacaswell
Copy link
Member Author

If we merge this I will write up a tracking issue with checklist of the methods + links etc for tomorrow.

N = 6
fig, ax = plt.subplots()
x = np.array([datetime.datetime(2023, 1, n) for n in range(1, N)])
ax.plot(x, range(1, N))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you also test ax.plot(range(1, N), x) and/or ax.plot(x, x)? It might be good to give an example of what to do when multiple arguments accept units.

timedeltas make sense
@tacaswell
Copy link
Member Author

so

which....looks not great

@oscargus
Copy link
Contributor

I think we can skip psd etc. I cannot really see a way to use dates there.

@tacaswell
Copy link
Member Author

so

That looks better.

@story645 story645 merged commit 04e1332 into matplotlib:main Sep 22, 2023
40 checks passed
@tacaswell tacaswell deleted the tst/dt_skel branch September 22, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants