Skip to content
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: Updated the docstring of pandas.Series.str.get #20667

Merged
merged 3 commits into from Apr 24, 2018

Conversation

pranavsuri
Copy link
Contributor

Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):

  • PR title is "DOC: update the docstring"
  • The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
  • The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
  • The html version looks good: python doc/make.py --single <your-function-or-method>
  • It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:

################################################################################
###################### Docstring (pandas.Series.str.get)  ######################
################################################################################

Extract the element from each component at the specified position.

Extract element from lists, tuples, or strings in each element in the
Series/Index.

Parameters
----------
i : int
    Position of the element to extract.

Returns
-------
items : Series/Index of objects

Examples
--------
>>> s = pd.Series(["String", (1, 2, 3), ["a", "b", "c"], 123])
>>> s
0       String
1    (1, 2, 3)
2    [a, b, c]
3          123
dtype: object

>>> s.str.get(1)
0      t
1      2
2      b
3    NaN
dtype: object

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
	See Also section not found

1 2
2 b
3 NaN
dtype: object
Copy link
Member

Choose a reason for hiding this comment

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

Looks good. May be we could a dictionary to the Series too, and an example indexing by a negative number. I'm not sure, as the example could be too complex for the simplicity of the function. But it'd illustrate the different behaviors.

It'd also help to have an explanation before the example, saying something like "When the value is not an iterable, or a dict where the index is not in the keys, str.get returns NaN".

If you do it you may encounter this bug: #20671. I'd wait until it's fixed, otherwise you'd have to use an index that is a key in the dictionary, and not illustrate the case when the index is not a key, which raises a KeyError right now.

@jreback jreback added Docs Strings String extension data type and string data labels Apr 14, 2018
@pranavsuri
Copy link
Contributor Author

Hello @datapythonista! I see that you've fixed the bug (which I tried fixing, but your solution is a lot better). Should I proceed writing the docstring with a dictionary & a negative value?

@datapythonista
Copy link
Member

You can add it, yes. My changes are not merged, so when you run the validation script you'll get the doctest fail, but no problem with that, when the bug is fixed your doctest will pass and your PR could be merged.

Added an example with a dictionary, a negative integer and an example with a negative index, i.e., not present in the dictionary.
@pep8speaks
Copy link

pep8speaks commented Apr 24, 2018

Hello @pranavsuri! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on April 24, 2018 at 09:56 Hours UTC

@codecov
Copy link

codecov bot commented Apr 24, 2018

Codecov Report

Merging #20667 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20667      +/-   ##
==========================================
+ Coverage   91.81%   91.83%   +0.02%     
==========================================
  Files         153      153              
  Lines       49270    49318      +48     
==========================================
+ Hits        45238    45293      +55     
+ Misses       4032     4025       -7
Flag Coverage Δ
#multiple 90.23% <ø> (+0.02%) ⬆️
#single 41.87% <ø> (-0.03%) ⬇️
Impacted Files Coverage Δ
pandas/core/strings.py 98.32% <ø> (ø) ⬆️
pandas/core/nanops.py 95.13% <0%> (-1.17%) ⬇️
pandas/core/arrays/categorical.py 95.78% <0%> (-0.41%) ⬇️
pandas/core/indexes/base.py 96.63% <0%> (-0.05%) ⬇️
pandas/io/pytables.py 92.41% <0%> (-0.05%) ⬇️
pandas/tseries/offsets.py 97% <0%> (-0.01%) ⬇️
pandas/core/indexes/datetimelike.py 96.72% <0%> (ø) ⬆️
pandas/core/resample.py 96.43% <0%> (ø) ⬆️
pandas/core/base.py 96.79% <0%> (ø) ⬆️
pandas/core/arrays/base.py 84.14% <0%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2794474...26fdb84. Read the comment docs.

@datapythonista
Copy link
Member

@pranavsuri, Can you fix the PEP-8 issues? Besides that lgtm. But it needs to wait until #20671 is merged.

@jreback jreback added this to the 0.23.0 milestone Apr 24, 2018
@jreback jreback merged commit 0ae19a1 into pandas-dev:master Apr 24, 2018
@jreback
Copy link
Contributor

jreback commented Apr 24, 2018

thanks @pranavsuri

@pranavsuri pranavsuri deleted the str-get-fix branch April 28, 2018 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Strings String extension data type and string data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants