-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
BUG: Series.map({}) should retain the original dtype #18509
Comments
@jschendel yeah [7], and [8] 'work' but should be the correct type (bug I introduced). The others are not well tested (cat / interval), so not surprising they are wrong. [6] also not tested well. |
Repeating my comment from #18491 here: I am not sure I find this special casing a good idea. I would rather keep Do you have a specific use case for this? (for wanting to be smart?) |
To further clarify: I think we are just trying to be too smart here, introducing a lot of corner cases. I would say: just let the user handle this (if he/she wants to retain the index type, he/she can simply wrap the map call in the appropriate index type). So what is the rationale of trying to keep the index type in case of all-NaNs ? Because you think a user typically uses
In that case I, unexpectedly, get a DatetimeIndex instead of a FloatIndex. To put it differently, we wouldn't distinguish a NaN and NaT return:
Also, you added a special case of trying to preserve uint64 data type for Uint64Index. Shouldn't we then do this for Series with different dtypes as well? And about [1] (first example at the top) being a bug: this has always been the behaviour, so if we change this I would see it as a API breaking change rather than a bug fix. |
#18491 (comment)
[1] should infer to a datetime64 as well
@jschendel comments
On master:
IntervalIndex
,CategoricalIndex
, andIndex
withobject
dtype get coerced toFloat64Index
:PeriodIndex
andTimedeltaIndex
get coercedDatetimeIndex
:The text was updated successfully, but these errors were encountered: