Skip to content

DOC: Updated docstring for Series.str.cat (pandas-dev#35556) #35784

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

Closed
wants to merge 17 commits into from

Conversation

souris-dev
Copy link
Contributor

@souris-dev souris-dev commented Aug 18, 2020

I have updated the docstring for Series.str.cat to make the issue in #35556 more explicit to the user.

@souris-dev
Copy link
Contributor Author

I tried to figure out the cause of the two Azure pipeline tests failing, but could not understand how that would be caused by a change in the docstring.
I'd really appreciate some help or hints regarding this, thanks.

@WillAyd WillAyd added the Docs label Aug 18, 2020
@MarcoGorelli
Copy link
Member

Hi @souris-dev - this is off to a good start, if it's still active could you address Will's comments please?

@souris-dev
Copy link
Contributor Author

My apologies for the tardiness (was a bit busy the past week), I'll make the changes and commit in a day. Thanks.

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell this is correct and addresses Will's comment, I've just made an absolutely minor (non-blocking) comment

@@ -2466,7 +2474,27 @@ def cat(self, others=None, sep=None, na_rep=None, join="left"):
2 -c
dtype: object

For more examples, see :ref:`here <text.concatenate>`.
When `others` is a Series/Index/DataFrame, it is advisable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to explain why one should do this. e.g. to have alignment since the join is defaulting to left.

show an example first with index and index, then index and series of the index and it should be more clear.

For more examples, see :ref:`here <text.concatenate>`.
When `others` is a Series/Index/DataFrame, it is advisable
to use `to_numpy()` while passing it to the function to
avoid unexpected results.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the 'unexpected results'.

Notes
-----
Since version 1.0.0, index alignment is performed when
`others` is a Series/Index/DataFrame (or a list-like containing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this true for an Index? If the behavior is anything like arithmetic, we would do alignment for Series/DataFrame, but Index would behave more like ndarray

to use `to_numpy()` while passing it to the function to
avoid unexpected results.

The following example demostrates this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the next paragraph both end in colons. is this paragraph needed?


>>> idx = pd.Index(["a", "b", "c", "d", "e"])
>>> ser = pd.Series(["f", "g", "h", "i", "j"])
>>> idx.str.cat(ser) # the caller is an Index here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more space before the #

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Oct 24, 2020
Copy link
Member

@arw2019 arw2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@souris-dev are you interested in continuing? If yes address comments and we'll take another look

@souris-dev
Copy link
Contributor Author

souris-dev commented Nov 2, 2020

Sorry for the delay, I'm in the process of addressing the comments and will update soon.

@arw2019
Copy link
Member

arw2019 commented Dec 11, 2020

Closing for now. @souris-dev let us know whenever you'd like to continue and we'll reopen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: .str.cat output in case of Index object
6 participants