-
-
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
DOC: fix mistake in Series.str.cat #21330
DOC: fix mistake in Series.str.cat #21330
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21330 +/- ##
=======================================
Coverage 91.85% 91.85%
=======================================
Files 153 153
Lines 49549 49549
=======================================
Hits 45512 45512
Misses 4037 4037
Continue to review full report at Codecov.
|
pandas/core/strings.py
Outdated
if the calling Series/Index is categorical. | ||
concat : | ||
str if `others` is None, Series/Index of objects if `others` is not | ||
None. |
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 would still put all possible data-types right after the
concat:
part. -
OCD kicking in here: can we rephrase so that
None
isn't by itself on that last line?
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.
@h-vetinari : Moving your reply here to the thread:
@gfyoung
It doesn't fit, unfortunately. Otherwise I would have left it there as well.
I'm guessing this is mainly due to the docstring parser + linelength-limit; it might work without a line break - I could do a #noqa
line, I guess?
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.
-
The
dtype
suggestion I made should be do-able on one line. -
For these types of issues with a single word on one line, you can try rewriting the whole description e.g.:
If `other` is None, `str` is returned, otherwise `Series / Index` of objects is returned.
We generally try avoid using noqa
unless absolutely necessary.
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.
Fair enough. See new suggestion
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.
Nice! LGTM!
@h-vetinari Thanks! |
(cherry picked from commit 0c65c57)
(cherry picked from commit 0c65c57)
Fix error in API-docstring that was introduced at the end of #20347 due to timepressure for the v.0.23-cutoff: removed functionality that categorical callers get categorical output, but forgot to adapt doc-string.
Unfortunately, this survived both #20347 and the follow-up, but since v.0.23.1 is coming soon, I didn't wanna let this opportunity pass.