-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: updated the docstring of Series.dot #22890
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 Series.dot #22890
Conversation
Hello @HubertKl! Thanks for submitting the PR.
|
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.
Great PR, added some comments, just minor things.
pandas/core/series.py
Outdated
|
||
Returns | ||
------- | ||
dot_product : scalar or Series | ||
scalar, Series or numpy.ndarray | ||
return the dot product of the Series and other if other is a |
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 capitalize Return
pandas/core/series.py
Outdated
>>> ser3 = pd.Series({'a': 0, 'b': 1, 'c': 2, 'd': 3}) | ||
>>> ser4 = pd.Series({'d': 0, 'c': 1, 'b': 2, 'a': 3}) | ||
>>> ser3.dot(ser4) | ||
4 |
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 I'd remove this last example and the previous comment. While I think it can be very useful, I think that applies to every method of Series and DataFrame, and I don't think we want to show this in every docstring.
dtype: int64 | ||
>>> arr = [[0,1],[-2,3],[4,-5],[6,7]] | ||
>>> ser.dot(arr) | ||
array([24, 14]) |
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 it probably makes more sense to show an example with a numpy array, more than with Python lists. I don't think (may be I'm wrong) in real cases it's so common to make operations between Python lists and pandas objects.
But I'll leave that to you, whatever you think it makes more sense.
pandas/core/series.py
Outdated
Examples | ||
-------- | ||
>>> ser = pd.Series([0,1,2,3]) | ||
>>> ser2 = pd.Series([-1,2,-3,4]) |
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 use s
for the first Series, and other
for the second instead? We try to keep them consistent in all docstrings. Or if you think it makes sense, weights
instead of other
, I think that will help at least people with a machine learning background (but if you use weights
probably better to use floats for the values).
Also, can you review PEP-8 of the whole examples? I see that after most commas you're missing a space.
pandas/core/series.py
Outdated
|
||
See Also | ||
-------- | ||
DataFrame.dot: Compute dot product with the columns of the DataFrame. |
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.
Not sure if I'd add Series.mul
here. Up to you.
Codecov Report
@@ Coverage Diff @@
## master #22890 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 169 169
Lines 50835 50835
=======================================
Hits 46868 46868
Misses 3967 3967
Continue to review full report at Codecov.
|
Thanks @datapythonista for the comments! I updated the pull request. I did check PEP8 with the command |
The thing with PEP-8, is that |
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.
lgtm, thanks @HubertKl
If you want, may be it would be useful to show in the examples s @ other
too.
Thanks for the advice, it looks even better now. I added the s @ other example. |
Thanks @HubertKl |
…xamples * repo_org/master: (66 commits) CLN: doc string (pandas-dev#23469) DOC: Add cookbook entry for triangular correlation matrix (GH22840) (pandas-dev#23032) add number of Errors, Warnings to scripts/validate_docstrings.py (pandas-dev#23150) BUG: Allow freq conversion from dt64 to period (pandas-dev#23460) ENH: Add FrozenList.union and .difference (pandas-dev#23394) REF: cython cleanup, typing, optimizations (pandas-dev#23464) strictness and checks for Timedelta _simple_new (pandas-dev#23433) Fixing flake8 problems new to flake8 3.6.0 (pandas-dev#23472) DOC: Updating the docstring of Series.dot (pandas-dev#22890) TST: Fixturize series/test_analytics.py (pandas-dev#22755) BUG/ENH: Handle NonexistentTimeError in date rounding (pandas-dev#23406) PERF: speed up concat on Series by making _get_axis_number() a classmethod (pandas-dev#23404) REF: Remove DatetimelikeArrayMixin._shallow_copy (pandas-dev#23430) REF: strictness/simplification in DatetimeArray/Index _simple_new (pandas-dev#23431) REF: cython cleanup, typing, optimizations (pandas-dev#23456) TST: tweak Hypothesis configuration and idioms (pandas-dev#23441) BUG: fix HDFStore.append with all empty strings error (GH12242) (pandas-dev#23435) TST: Skip 32bit failing IntervalTree tests (pandas-dev#23442) BUG: Deprecate nthreads argument (pandas-dev#23112) style: fix import format at pandas/core/reshape (pandas-dev#23387) ...
git diff upstream/master -u -- "*.py" | flake8 --diff