Uint Series can be upcast to int64 unexpectedly

[chaburkland]

Description

When using certain operations that insert fill_values, uint Series can be unexpectedly upcast to int64.

Example

s = sf.Series.from_element(1, index=range(5), dtype=np.uint8)
assert s.dtype == np.uint8

s = s.reindex(range(6), fill_value=0)
assert s.dtype == np.int64

# The way to get back the expected dtype (without calling astype) is to do:
s = s.reindex(range(6), fill_value=np.uint8(0))
assert s.dtype == np.uint8

Discussion

If updating the code to perform more specific type checks and/or edge-case handling handling proves to be too expensive or complicated to implement, it's likely worth ignoring. I'm creating this ticket as I found it to be an interesting dtype conversion edge-case I stumbled across when working with uints.

Platform

Run the following function (static-frame >= 0.8.1) and provide the results to define your platform and environment:

<Series: platform>
<Index>
platform           Linux-5.4.0-87-generic-x86_64-with-glibc2.27
sys.version        3.8.12 (default, Oct 18 2021, 17:34:02) [GCC 7.5.0]
static-frame       0.8.20
numpy              1.17.4
pandas             0.24.2
xlsxwriter         1.1.5
openpyxl           2.5.4
xarray             <ModuleNotFoundError>
tables             3.6.1
pyarrow            0.17.0
msgpack            <ModuleNotFoundError>
msgpack_numpy      <ModuleNotFoundError>
<<U13>             <object>

[flexatone]

Thanks for posting this issue.

The issue here is that with any usage of fill_value we resolve the dtype between the destination of the fill and the value to be filled. As a Python integer by default is interpreted as an int64 (on modern OSs), resolving the final dtype to int64 is correct; the only recourse is to do as you have shown and explicitly pass a uint8 as fill_value.

The reason I think there is no other option is that the function should not behave differently between fill_values of 0 or np.int64(0): both have to be interpreted as int64, and both should upcast the array to int64. Do you see any other option?

[chaburkland]

I see. I guess what I envisioned is something like this:

def resolve_dtype(fill_value, dtype):
    # Handle cases where fill_value fits inside the existing dtype
    if isinstance(fill_value, int) and np.iinfo(dtype).min <= fill_value <= np.iinfo(dtype).max:
        return dtype
    ....

That's what I meant by "this check might be too expensive", since you'd be forced to do a bunch of explicit examinations on the incoming value to determine where it fits.

[flexatone]

That is an interesting approach, but it is quite different than the how dtype resolution is implemented in StaticFrame. This would resolve dtype by value and type, not type alone. I am not sure this more subtle approach is more intuitive than the current approach. And it would lead to interesting questions, such as: if inserting an int64 1 into a Boolean array, should the array remain Boolean?

[chaburkland]

I think you could still leave the existing type resolution paradigm alone, instead extending it a bit to allow for specific edge cases (i.e. this issue). My sketch above wasn't very good, since it forced a re-write of the existing dtype resolution function. This is closer to what I was envisioning:

def resolve_dtype(dtype1, dtype2, fill_value=sentinel)
     # Leave all the existing logic the same, just add some additional branches to handle very specific edge cases

Admittedly, I don't like this very much, because I think it adds far too much complexity to address a rather esoteric edge case that already has a solution