-
-
Notifications
You must be signed in to change notification settings - Fork 151
feat(arithmetic): #1347 #1378 ➗ truediv #1431
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
Conversation
…e/cmp0xff/truediv
…e/cmp0xff/truediv
…e/cmp0xff/truediv
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.
Biggest issue here is the use of Index[Timedelta]
, which I would prefer if we could avoid in the stubs, even though it can be created via some non-standard usage.
I would say that a community decision can clarify the situation, if you would help 🙂 |
That might take a while. But in If people complain that the stubs don't work because they are creating code that is essentially a |
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.
Everything looks fine. The only thing I'm not sure about - and this might just be a Github issue - is that the directory tests/indexes/arithmetic/timedelta
is not showing as being removed from the repo. I see that it is not present in your branch in your repo.
Can you tell if you did an explicit commit that has git rm tests/indexes/arithmetic/timedelta
?
If you can't tell, I can approve and merge, and if it is still there, then do a separate PR to fix that.
I cannot delete it any further. > git rm .\tests\indexes\arithmetic\timedelta
fatal: pathspec '.\tests\indexes\arithmetic\timedelta' did not match any files I think |
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.
thanks @cmp0xff
OK. I approved and merged and the folder is gone. But the review process indicated it was still there. Hence the confusion |
TimedeltaIndex
andDatetimeIndex
#1378