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: Fix Order of parameters in docstrings #23611

Merged
merged 5 commits into from
Nov 11, 2018

Conversation

thoo
Copy link
Contributor

@thoo thoo commented Nov 10, 2018

Fix the following:

pandas.io.json.json_normalize: Wrong parameters order. Actual: ('data', 'record_path', 'meta', 'meta_prefix', 'record_prefix', 'errors', 'sep'). Documented: ('data', 'record_path', 'meta', 'record_prefix', 'meta_prefix', 'errors', 'sep')
pandas.crosstab: Wrong parameters order. Actual: ('index', 'columns', 'values', 'rownames', 'colnames', 'aggfunc', 'margins', 'margins_name', 'dropna', 'normalize'). Documented: ('index', 'columns', 'values', 'aggfunc', 'rownames', 'colnames', 'margins', 'margins_name', 'dropna', 'normalize')
pandas.Series.rolling: Wrong parameters order. Actual: ('window', 'min_periods', 'center', 'win_type', 'on', 'axis', 'closed'). Documented: ('window', 'min_periods', 'center', 'win_type', 'on', 'closed', 'axis')
pandas.DataFrame.rolling: Wrong parameters order. Actual: ('window', 'min_periods', 'center', 'win_type', 'on', 'axis', 'closed'). Documented: ('window', 'min_periods', 'center', 'win_type', 'on', 'closed', 'axis')
-pandas.DataFrame.to_html: Wrong parameters order. Actual: ('buf', 'columns', 'col_space', 'header', 'index', 'na_rep', 'formatters', 'float_format', 'sparsify', 'index_names', 'justify', 'bold_rows', 'classes', 'escape', 'max_rows', 'max_cols', 'show_dimensions', 'notebook', 'decimal', 'border', 'table_id'). Documented: ('buf', 'columns', 'col_space', 'header', 'index', 'na_rep', 'formatters', 'float_format', 'sparsify', 'index_names', 'justify', 'max_rows', 'max_cols', 'show_dimensions', 'bold_rows', 'classes', 'escape', 'notebook', 'decimal', 'border', 'table_id')
-pandas.DataFrame.to_string: Wrong parameters order. Actual: ('buf', 'columns', 'col_space', 'header', 'index', 'na_rep', 'formatters', 'float_format', 'sparsify', 'index_names', 'justify', 'line_width', 'max_rows', 'max_cols', 'show_dimensions'). Documented: ('buf', 'columns', 'col_space', 'header', 'index', 'na_rep', 'formatters', 'float_format', 'sparsify', 'index_names', 'justify', 'max_rows', 'max_cols', 'show_dimensions', 'line_width')
pandas.tseries.offsets.CustomBusinessDay: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.tseries.offsets.CustomBusinessMonthEnd: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.tseries.offsets.CustomBusinessMonthBegin: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.tseries.offsets.CBMonthEnd: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.tseries.offsets.CBMonthBegin: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.tseries.offsets.CDay: Wrong parameters order. Actual: ('n', 'normalize', 'weekmask', 'holidays', 'calendar', 'offset'). Documented: ('n', 'offset', 'normalize', 'weekmask', 'holidays', 'calendar')
pandas.testing.assert_series_equal: Wrong parameters order. Actual: ('left', 'right', 'check_dtype', 'check_index_type', 'check_series_type', 'check_less_precise', 'check_names', 'check_exact', 'check_datetimelike_compat', 'check_categorical', 'obj'). Documented: ('left', 'right', 'check_dtype', 'check_index_type', 'check_series_type', 'check_less_precise', 'check_exact', 'check_names', 'check_datetimelike_compat', 'check_categorical', 'obj')```

@pep8speaks
Copy link

Hello @thoo! Thanks for submitting the PR.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

generally you cannot change the order of the kwargs themselves
just fix the doc strings. if it makes more sense and it has a lot of args then we need a note in the whatsnew

@codecov
Copy link

codecov bot commented Nov 10, 2018

Codecov Report

Merging #23611 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #23611   +/-   ##
=======================================
  Coverage   92.25%   92.25%           
=======================================
  Files         161      161           
  Lines       51277    51277           
=======================================
  Hits        47305    47305           
  Misses       3972     3972
Flag Coverage Δ
#multiple 90.63% <ø> (ø) ⬆️
#single 42.32% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/reshape/pivot.py 96.55% <ø> (ø) ⬆️
pandas/core/window.py 96.4% <ø> (ø) ⬆️
pandas/tseries/offsets.py 97.22% <ø> (ø) ⬆️
pandas/util/testing.py 86.71% <ø> (ø) ⬆️
pandas/io/json/normalize.py 96.87% <ø> (ø) ⬆️

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 8ed92ef...5d7c5ce. Read the comment docs.

* upstream/master:
  TST: Use intp as expected dtype in IntervalIndex indexing tests (pandas-dev#23609)
@datapythonista
Copy link
Member

@thoo I think in this case it makes more sense to change the order of the parameters, as you did initially. But let's do that in a separate PR, just leave the other changes here.

@datapythonista
Copy link
Member

I created #23612 to address the changes in to_string and to_html.

@@ -472,6 +472,7 @@ class Window(_Window):
on : string, optional
For a DataFrame, column on which to calculate
the rolling window, rather than the index
axis : int or string, default 0
Copy link
Member

Choose a reason for hiding this comment

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

when specifying parameter types: string --> str (a couple other instances of this elsewhere too)

normalize : bool, default False
Normalize start/end dates to midnight before generating date range
weekmask : str, Default 'Mon Tue Wed Thu Fri'
weekmask of valid business days, passed to ``numpy.busdaycalendar``
holidays : list
list/array of dates to exclude from the set of valid business days,
passed to ``numpy.busdaycalendar``
calendar : pd.HolidayCalendar or np.busdaycalendar
calendar : pd.HolidayCalendar or np.
Copy link
Member

Choose a reason for hiding this comment

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

looks like np.busdaycalendar accidentally got truncated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry !

@jschendel jschendel added the Docs label Nov 10, 2018
@jschendel jschendel added this to the 0.24.0 milestone Nov 10, 2018
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @thoo for the work on this

@datapythonista datapythonista changed the title Fix Order of parameters in docstrings DOC: Fix Order of parameters in docstrings Nov 10, 2018
@jreback jreback merged commit 00ca0f9 into pandas-dev:master Nov 11, 2018
@jreback
Copy link
Contributor

jreback commented Nov 11, 2018

thanks @thoo nice work!

@datapythonista is this check now automatic in code_checks? (if not, pls create an issue for this, if you don't already).

@datapythonista
Copy link
Member

@jreback I'm adding the docstring checks that are ready to #23614 which still requires some work.

I'll add the validation of the order the parameters, but there too it'll still fail, until #23614 is merged.

thoo added a commit to thoo/pandas that referenced this pull request Nov 11, 2018
* upstream/master:
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
  DOC: Fixes to docstring to add validation to CI (pandas-dev#23560)
  DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600)
  MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592)
  DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
thoo added a commit to thoo/pandas that referenced this pull request Nov 12, 2018
…fixed

* upstream/master:
  DOC: Enhancing pivot / reshape docs (pandas-dev#21038)
  TST: Fix xfailing DataFrame arithmetic tests by transposing (pandas-dev#23620)
  BUILD: Simplifying contributor dependencies (pandas-dev#23522)
  BUG/REF: TimedeltaIndex.__new__ (pandas-dev#23539)
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
@thoo thoo deleted the order_of_parameter branch January 2, 2019 20:26
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Fix order of parameters in the docstrings
5 participants