Skip to content

Conversation

it176131
Copy link
Contributor

@it176131 it176131 commented Mar 26, 2025

	- Added test :callable:`test_pivot_table_index_and_column_keys_with_nan` to test that null index/column keys are kept when `dropna=False`, and removed when `dropna=True`.
	- Updated docs for `dropna` parameter in :callable:`pivot_table`.
@it176131
Copy link
Contributor Author

Which doc/source/whatsnew/vX.X.X.rst file should I edit? v2.3.0.rst?

	- Replaced `np.NaN` with `np.nan` to pass failing tests on CI build.
	- Added comma and period to `dropna` parameter in :callable:`pivot_table` to pass docstring validation hook in CI build.
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Which doc/source/whatsnew/vX.X.X.rst file should I edit? v2.3.0.rst?

No need to add to the whatsnew when improving a docstring.

Comment on lines 105 to 107
- rows with a NaN value in any column will be omitted before computing margins,
- index/column keys containing NA values will be dropped (see ``dropna``
parameter in ``pandas.DataFrame.groupby``).
Copy link
Member

Choose a reason for hiding this comment

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

You'll also need to update the entry in shared_docs within pandas.core.frame.

Copy link
Member

Choose a reason for hiding this comment

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

Can you use an NA value instead of a NaN value. Also link to the groupby via :meth:`DataFrame.groupby` instead of pandas.DataFrame.groupby.

Copy link
Member

Choose a reason for hiding this comment

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

It's :meth: and not :method: for sphinx.

@rhshadrach rhshadrach added Docs Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Mar 28, 2025
	- Updated reference link to `pandas.DataFrame.groupby` with :method: syntax in the :callable:`pivot_table` param `dropna` docstring.
	- Replaced "NaN" with "NA" per request from @rhshadrach in comment.
	- Updated value of :attr:`DataFrame._shared_docs["pivot_table"]` to include updated docstring from `pandas.core.reshape.pivot.pivot_table`.
@it176131 it176131 requested a review from rhshadrach March 28, 2025 19:31
it176131 and others added 9 commits March 30, 2025 11:43
	- Updated spelling of :method: to :meth: per comment feedback.
	- Updated shared docs replacing :method: with :meth: per comment feedback.
	- Removed docstring for `test_pivot_table_index_and_column_keys_with_nan` per comment feedback.
	- Renamed `actual` -> `result` per comment feedback.
	- Removed type hints for `test_pivot_table_index_and_column_keys_with_nan` per comment feedback.
	- Removed data manipulations from `expected` to prevent dependencies on other pandas methods.
	- Added comment with GitHub issue reference number.
	- Updated _shared_docs["pivot_table"] to match docstring in pandas/core/reshape/pivot.py.

modified:   pandas/core/reshape/pivot.py
	- Updated `dropna` param docstring in `pivot_table` so the dashes convert to a bulleted list.
@it176131 it176131 requested a review from rhshadrach March 31, 2025 01:04
@it176131
Copy link
Contributor Author

it176131 commented Mar 31, 2025

@rhshadrach it looks like two warnings are raised because of the bulleted list formatting: (1), (2). Is there another way you recommend formatting the docstring? Or do you have an example with a bulleted list that I can work from?

Think I figured it out in 1a82d2a.

it176131 and others added 2 commits March 31, 2025 10:25
	- Updated dropna param description in `_shared_docs["pivot_table"]` to use bulleted list similar to `parse_dates` param description in `pandas.io.parsers.readers._doc_read_csv_and_table`.

modified:   pandas/core/reshape/pivot.py
	- Updated docstring for `dropna` param in :func:`pivot_table` to use bulleted list similar to `parse_dates` param description in `pandas.io.parsers.readers._doc_read_csv_and_table`.
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looking good!

	- Updated `_shared_docs["pivot_table"]` value per comments.

modified:   pandas/core/reshape/pivot.py
	- Updated `dropna` param docstring in :func:`pivot_table` per comments.
@it176131 it176131 requested a review from rhshadrach April 1, 2025 01:37
@it176131
Copy link
Contributor Author

it176131 commented Apr 2, 2025

@rhshadrach ready for review

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach changed the title Doc pivot table DOC: Add details of dropna in DataFrame.pivot_table Apr 2, 2025
@rhshadrach rhshadrach added this to the 3.0 milestone Apr 2, 2025
@rhshadrach rhshadrach merged commit 7e4d306 into pandas-dev:main Apr 2, 2025
42 checks passed
@rhshadrach
Copy link
Member

Thanks @it176131 - nice work!

@it176131 it176131 deleted the doc_pivot_table branch April 3, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docs Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: pivot_table drops rows and columns despite values being non-NaN

2 participants