-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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 may raise TypeError in Categorical or DatetimeTz #12532
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
is_categorical_dtype, | ||
_possibly_cast_to_datetime, | ||
_possibly_castable, _possibly_convert_platform, | ||
_try_sort, is_internal_type, is_datetimetz, | ||
_try_sort, is_extension_type, is_datetimetz, | ||
_maybe_match_name, ABCSparseArray, | ||
_coerce_to_dtype, SettingWithCopyError, | ||
_maybe_box_datetimelike, ABCDataFrame, | ||
|
@@ -2063,28 +2063,33 @@ def map(self, arg, na_action=None): | |
y : Series | ||
same index as caller | ||
""" | ||
values = self.asobject | ||
|
||
if na_action == 'ignore': | ||
mask = isnull(values) | ||
|
||
def map_f(values, f): | ||
return lib.map_infer_mask(values, f, mask.view(np.uint8)) | ||
if is_extension_type(self.dtype): | ||
values = self._values | ||
if na_action is not None: | ||
raise NotImplementedError | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, testing this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
map_f = lambda values, f: values.map(f) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the background of this suggestion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I just thought it would be a cleaner way. |
||
else: | ||
map_f = lib.map_infer | ||
values = self.asobject | ||
|
||
if na_action == 'ignore': | ||
def map_f(values, f): | ||
return lib.map_infer_mask(values, f, | ||
isnull(values).view(np.uint8)) | ||
else: | ||
map_f = lib.map_infer | ||
|
||
if isinstance(arg, (dict, Series)): | ||
if isinstance(arg, dict): | ||
arg = self._constructor(arg, index=arg.keys()) | ||
|
||
indexer = arg.index.get_indexer(values) | ||
new_values = algos.take_1d(arg._values, indexer) | ||
return self._constructor(new_values, | ||
index=self.index).__finalize__(self) | ||
else: | ||
mapped = map_f(values, arg) | ||
return self._constructor(mapped, | ||
index=self.index).__finalize__(self) | ||
new_values = map_f(values, arg) | ||
|
||
return self._constructor(new_values, | ||
index=self.index).__finalize__(self) | ||
|
||
def apply(self, func, convert_dtype=True, args=(), **kwds): | ||
""" | ||
|
@@ -2193,7 +2198,12 @@ def apply(self, func, convert_dtype=True, args=(), **kwds): | |
if isinstance(f, np.ufunc): | ||
return f(self) | ||
|
||
mapped = lib.map_infer(self.asobject, f, convert=convert_dtype) | ||
if is_extension_type(self.dtype): | ||
mapped = self._values.map(f) | ||
else: | ||
values = self.asobject | ||
mapped = lib.map_infer(values, f, convert=convert_dtype) | ||
|
||
if len(mapped) and isinstance(mapped[0], Series): | ||
from pandas.core.frame import DataFrame | ||
return DataFrame(mapped.tolist(), index=self.index) | ||
|
@@ -2779,7 +2789,7 @@ def _try_cast(arr, take_fast_path): | |
|
||
try: | ||
subarr = _possibly_cast_to_datetime(arr, dtype) | ||
if not is_internal_type(subarr): | ||
if not is_extension_type(subarr): | ||
subarr = np.array(subarr, dtype=dtype, copy=copy) | ||
except (ValueError, TypeError): | ||
if is_categorical_dtype(dtype): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2194,11 +2194,22 @@ def groupby(self, to_groupby): | |
------- | ||
groups : dict | ||
{group name -> group labels} | ||
|
||
""" | ||
return self._groupby(self.values, _values_from_object(to_groupby)) | ||
|
||
def map(self, mapper): | ||
""" | ||
Apply mapper function to its values. | ||
|
||
Parameters | ||
---------- | ||
mapper : callable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, so it seems that hmm, can you create an issue so we can look at this. might be a bit non-trivial to fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, let me issue a separate issue (to close #12756 also). |
||
Function to be applied. | ||
|
||
Returns | ||
------- | ||
applied : array | ||
""" | ||
return self._arrmap(self.values, mapper) | ||
|
||
def isin(self, values, level=None): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -468,6 +468,24 @@ def take(self, indices, axis=0, allow_fill=True, fill_value=None): | |
na_value=-1) | ||
return self._create_from_codes(taken) | ||
|
||
def map(self, mapper): | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe in another PR can consolidate the doc-strings of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Left it as it is, because |
||
Apply mapper function to its categories (not codes). | ||
|
||
Parameters | ||
---------- | ||
mapper : callable | ||
Function to be applied. When all categories are mapped | ||
to different categories, the result will be Categorical which has | ||
the same order property as the original. Otherwise, the result will | ||
be np.ndarray. | ||
|
||
Returns | ||
------- | ||
applied : Categorical or np.ndarray. | ||
""" | ||
return self.values.map(mapper) | ||
|
||
def delete(self, loc): | ||
""" | ||
Make new Index with passed location(-s) deleted | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be:
Index(new_categories).take(self._codes)
? maybe that's the sameThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the result should be the same. I chose numpy logic to omit
Index
creation, butIndex.take
is preferable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its fine, more of a style things, but yes should be the same