-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
ENH: improve extract and get_dummies methods for Index.str (fix for #9980) #9985
Conversation
result = Series([f(val)[0] for val in arr], | ||
name=_get_single_group_name(regex), | ||
index=arr.index, dtype=object) | ||
if isinstance(arr, Index): |
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.
can we just use StringMethods._wrap_result
here?
Thanks! Added "Closes ... " description to the body to automatically close the original issue. I'll update #9843 once behavior is decided. |
# only non-capturing groups | ||
f = lambda: s.str.extract('(?:[AB]).*') | ||
self.assertRaises(ValueError, f) | ||
# Index only works with one regex group since |
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.
can you add the issue reference here as a comment
pls add a release note. lgtm otherwise |
01d85cb
to
11e3b5a
Compare
@jreback updated with release note and issue number as inline comment as you suggested |
result = Index([f(val)[0] for val in arr], | ||
name=_get_single_group_name(regex), | ||
dtype=object) | ||
else: |
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.
I think its worthile to change _wrap_result
to something like this:
def _wrap_result(self, result, **kwargs):
if not isinstance(result, (Series, Index)):
result = type(self.series)(result, **kwargs)
.....
should work if expand=False
@mortada can you update/rebase. I believe you can use |
@jreback updated, I can squash the commits later but for now I want to showcase the difference between the two implementations. To simplify the code inside |
if 'name' in kwargs: | ||
name = kwargs['name'] | ||
else: | ||
name = getattr(result, 'name', None) or self.series.name |
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.
you can just do:
name = kwargs.get('name') or getattr(result,'name',None) or self.series.name
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.
good call ... will update
merged via e686387 thanks! |
Closes #9980