Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TST: Added regression test #31538
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
TST: Added regression test #31538
Changes from 16 commits
aa5dc3b
270dfee
5691066
2cf0756
33e24a3
bc75e25
20ba781
94cedb9
1cafb66
a5608eb
fd04076
1d17645
c454822
faf90e1
5cdd1d1
6fef7f8
799ae78
54ea5a0
808599b
0db9073
d62cad3
1449d8c
c5dc76c
6d2e72f
5742c46
951074f
fbce136
5954709
f51559f
e183a23
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
nitpick: time_object -> time_objects. otherwise i think this is ready
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.
should we be testing that all four of these ops raise TypeError?
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.
Yea this isn't very clear as is whether one or all of these raise a TypeError - can these be split into different contexts?
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.
Or alternately you can parametrize add, radd, sub, rsub
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 can do that, is that's the preferred way?
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.
dealer's choice. i tend not to parametrize over add/radd/sub/rsub since it repeats all the setup code for for what i see as little gain, but on a case-by-case basis if it improves clarity, go for it.