Switch to_timedelta argument type from list to Sequence #959
Conversation
| Sequence[str | float | timedelta] | ||
| | list[str | float | timedelta] |
There was a problem hiding this comment.
Combine these to one item of Sequence[str | float | timedelta]
There was a problem hiding this comment.
Indeed Sequence is encompassing list so might as well.
tests/test_timefuncs.py
Outdated
| td1: pd.Timedelta = pd.Timedelta("2 days") | ||
| r1: pd.TimedeltaIndex = tds1 + td1 | ||
| r2: pd.TimedeltaIndex = tds1 - td1 | ||
| r3: pd.TimedeltaIndex = tds1 * 4.3 | ||
| r4: pd.TimedeltaIndex = tds1 / 10.2 |
There was a problem hiding this comment.
You don't need these tests here.
You do need to do the test as follows:
check(assert_type(pd.to_timedelta(["1 day"]), pd.TimedeltaIndex), pd.TimedeltaIndex)There was a problem hiding this comment.
Thanks for the feedback and the clarity!
| @@ -31,8 +31,7 @@ def to_timedelta( | |||
| @overload | |||
| def to_timedelta( | |||
| arg: ( | |||
| Sequence[float | timedelta] | |||
| | list[str | float | timedelta] | |||
| Sequence[str | float | timedelta] | |||
There was a problem hiding this comment.
The CI is failing because a plain string will match Sequence[str] . So you should change this to
| Sequence[str | float | timedelta] | |
| SequenceNotStr | Sequence[float | timedelta] |
You can import SequenceNotStr from _typing
Also, you should set up your environment and test locally.
There was a problem hiding this comment.
Great that you are confirming my initial solution, '1 day' and ['1 day'] are both Sequence.
I forgot to rerun locally when I pushed the feedback last time, should be good now.
Thanks for all the insights on my first PR here!
There was a problem hiding this comment.
I had to force the typehint in the test, let me know if this is in the spirit.
Co-authored-by: Irv Lustig <[email protected]>
to_timedeltaargument type fromlisttoSequence#956assert_type()to assert the type of any return valueShould fix the issue in #956 with allowing the sequence of string type, I was not sure about the
assert_typestep above, please feel free to guide me on that one.