[REVIEW] Fix inplace update of data and add Series.update

[galipremsagar]

Fixes: #7187

This PR:

• Fixes inplace manipulation of columns.
• Introduces Series.update
• Fixes incorrect dtype handling in Frame.where

[kkraus14]

Fixes incorrect dtype handling in Frame.where[WIP]

Before you dive deep into this, can you explain the situation this is handling?

[galipremsagar]

Fixes incorrect dtype handling in Frame.where[WIP]

Before you dive deep into this, can you explain the situation this is handling?

So the issue here is we are not correctly handling the dtype situation, where we need to compare the self & other dtypes and decide what the destination dtype has to be, currently we straight away type-cast other to self.dtype:

>>> import cudf
>>> s = cudf.Series([False, True, True])
>>> other = cudf.Series([3.0, 4.5, 7.0])
>>> s.where([True, True, True], other)
0    False
1     True
2     True
dtype: bool
>>> s.where([False, False, False], other)
0    True
1    True
2    True
dtype: bool
>>> s.to_pandas().where([False, False, False], other.to_pandas())
0      3
1    4.5
2      7
dtype: object
>>> s.to_pandas().where([False, True, False], other.to_pandas())
0       3
1    True
2       7
dtype: object

We should perform the correct dtype calcaulation for numeric types and raise an error when there could be a situation of mixed object dtypes like in pandas. Does this sound okay by you?

[kkraus14]

We should perform the correct dtype calcaulation for numeric types and raise an error when there could be a situation of mixed object dtypes like in pandas. Does this sound okay by you?

Okay, that sounds good. What about the situation where self is int8 and someone passes other with int32. What would the result type be or should we throw?

[galipremsagar]

What about the situation where self is int8 and someone passes other with int32. What would the result type be or should we throw?

Yup that's when the common dtype finding logic should kick in and result in final dtype as int32, for example:

>>> s
0    3
1    4
2    5
dtype: int8
>>> other = pd.Series([10, 11, 12], dtype='int32')
>>> other
0    10
1    11
2    12
dtype: int32
>>> s.where([True, False, True], other)
0     3
1    11
2     5
dtype: int32

[kkraus14]

Yup that's when the common dtype finding will kick in and result in final dtype as int32

Is that the expected behavior for end users? I.E. if I did something like:

s = cudf.Series([1, 2, 3], dtype='int8')
other = cudf.Series([11, 12, 13], dtype='int32')
s.where([False, True, True], other)

The output would be int32 while the data still nicely fits in int8. Would this be expected behavior for an end user? There's also the inplace flag, which would then either error in the above case or we'd need to introspect the data in other to determine if it can properly fit in the dtype of s.

[galipremsagar]

The dtype selection is also based on inplace, where if inplace is True the type-cast is not being performed and the number is probably being allowed to overflow:

>>> s
0     3
1    11
2     5
dtype: int8
>>> other = pd.Series([10, 10000, 12120], dtype='int32')
>>> s.where([True, False, True], other, inplace=True)
>>> s
0     3
1    16
2     5
dtype: int8
>>> s.where([True, False, True], other, inplace=False)
0        3
1    10000
2        5
dtype: int32

1. If inplace=False, I think we can go ahead with changing of dtype based on both self & other.
2. If inplace=True, we can return self.dtype itself instead of up-casting(but raise a warning stating there could be potential data loss due to down-casting) as we allow down-casting:

>>> import cudf
>>> s = cudf.Series([10, 10000, 12120], dtype='int32')
>>> s
0       10
1    10000
2    12120
dtype: int32
>>> s.astype('int8')
0    10
1    16
2    88
dtype: int8

[kkraus14]

Sounds like a good path forward to me.

[shwina]

LGTM

[galipremsagar]

rerun tests

[kkraus14]

@gpucibot merge