Skip to content

Conversation

@jorisvandenbossche
Copy link
Member

Discovered while trying to fix test failures in #40482. Currently normal division with a float scalar works, but floordiv doesn't (returns garbage):

In [42]: arr = pd.timedelta_range(0, periods=3)

In [43]: pd.Series(arr) / 2.0
Out[43]: 
0   0 days 00:00:00
1   0 days 12:00:00
2   1 days 00:00:00
dtype: timedelta64[ns]

In [44]: pd.Series(arr) // 2.0
Out[44]: 
0                 0 days 00:00:00
1   55681 days 08:53:22.404319232
2   55733 days 11:53:22.031689728
dtype: timedelta64[ns]

In [45]: pd.__version__
Out[45]: '1.3.3'

It's only the scalar case; dividing with eg float array already works:

In [46]: pd.Series(arr) // np.array([2.0]*3)
Out[46]: 
0   0 days 00:00:00
1   0 days 12:00:00
2   1 days 00:00:00
dtype: timedelta64[ns]

I suppose the current code uses the workaround of asi8 and casting back to timedelta, because numpy didn't support floordiv in the past (didn't check which version started to implement this, will see what CI says)

  • whatsnew entry

@jorisvandenbossche jorisvandenbossche added Bug Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type labels Nov 15, 2021
@jorisvandenbossche jorisvandenbossche added this to the 1.4 milestone Nov 15, 2021
two / tdser

@pytest.mark.parametrize("two", [2, 2.0, np.array(2), np.array(2.0)])
def test_td64arr_floordiv_numeric_scalar(self, box_with_array, two):
Copy link
Member Author

Choose a reason for hiding this comment

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

We have versions above of this test for multiplication and division, so adding one for floordiv as well

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can you add a whatsnew note, comment and lgtm

@unpack_zerodim_and_defer("__floordiv__")
def __floordiv__(self, other):

# breakpoint()
Copy link
Contributor

Choose a reason for hiding this comment

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

can you remove

@jorisvandenbossche
Copy link
Member Author

Hmm, so apparently it already passes on all numpy versions we test. OK, will clean-up then :)

@jreback jreback merged commit 5cdde87 into pandas-dev:master Nov 15, 2021
@jorisvandenbossche jorisvandenbossche deleted the timedeltaarray-div-float branch November 15, 2021 15:21
# else will raise
result = self.asi8 // other
np.putmask(result, self._isnan, iNaT)
result = self._ndarray / other
Copy link
Member

Choose a reason for hiding this comment

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

why is this using div instead of floordiv?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I was stupid .. :), and because the test I added (or any other test we already have) actually don't catch this because only use such data that it is the same for div or floordiv ..

Copy link
Member Author

Choose a reason for hiding this comment

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

-> #44471

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

Labels

Bug Numeric Operations Arithmetic, Comparison, and Logical operations Timedelta Timedelta data type

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants