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 test_eventplot to test_datetime.py #27466

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

shriyakalakata
Copy link
Contributor

@shriyakalakata shriyakalakata commented Dec 7, 2023

PR summary

I have added a datetime smoke test/example code under the test_eventplot method in lib/matplotlib/test/test_datetime.py. This addresses the Axes.eventplot task from #26864.

(edited) The plots generated from the latest test code:
eventplot

PR checklist

Copy link

@github-actions github-actions bot left a 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.

lib/matplotlib/tests/test_datetime.py Outdated Show resolved Hide resolved
lib/matplotlib/tests/test_datetime.py Outdated Show resolved Hide resolved
Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

On a side note, you should probably set your email address in your git config. The first commit is using a random DHCP-supplied hostname from NYU, and the second is using your default macbook hostname.

lib/matplotlib/tests/test_datetime.py Show resolved Hide resolved
@ksunden
Copy link
Member

ksunden commented Dec 8, 2023

I think we may wish to add a testcase which exercise the units behavior on the y axis (i.e. lineoffset and linelength params, set to datetime and timedelta, respectively)

While I think the test cases here are perfectly good for the more ordinary use of eventplot, checking that those do the sensible thing is part of what we want.

@shriyakalakata
Copy link
Contributor Author

I think we may wish to add a testcase which exercise the units behavior on the y axis (i.e. line offset and linelength params, set to datetime and timedelta, respectively)

I'm able to use datetime for lineoffset on the y axis but seems like linelength can't be of type timedelta

@QuLogic
Copy link
Member

QuLogic commented Dec 9, 2023

I'm able to use datetime for lineoffset on the y axis but seems like linelength can't be of type timedelta

I've opened #27479 to track this.

@QuLogic QuLogic added this to the v3.9.0 milestone Dec 9, 2023
Copy link
Member

@ksunden ksunden left a comment

Choose a reason for hiding this comment

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

Since linelengths is not well supported (and we have an issue to track getting it supported) I'll go ahead and approve this for what it is. We can edit it to include linelengths as timedeltas when that has been addressed.

@QuLogic QuLogic merged commit 87e09e8 into matplotlib:main Dec 11, 2023
40 checks passed
@QuLogic
Copy link
Member

QuLogic commented Dec 11, 2023

Thanks @shriyakalakata! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants