ENH: check for string and convert to list in DataFrame.dropna subset argument #41022

erfannariman · 2021-04-18T18:04:22Z

closes ENH: dropna should accept single column as string in subset argument #41021
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

pandas/core/frame.py

doc/source/whatsnew/v1.2.5.rst

pandas/tests/frame/methods/test_dropna.py

pandas/core/frame.py

erfannariman · 2021-04-19T12:01:24Z

@phofl @mzeitlin11 the test pandas/tests/reductions/test_reductions.py::TestSeriesReductions::test_all_any_params runs succesfully on my machine, but fails here in CI. So cannot reproduce the error to check what is wrong.

edit: my branch is up to date with master, so that cannot be the problem.

phofl · 2021-04-19T12:08:36Z

You can ignore the ResourceWarning tests. But pre commit and ci checks is real

phofl · 2021-04-19T12:10:49Z

pandas/core/frame.py

@@ -5803,7 +5805,7 @@ def dropna(
        axis: Axis = 0,
        how: str = "any",
        thresh=None,
-        subset=None,
+        subset: Optional[Union[Hashable, Sequence[Hashable]]] = None,


Could you type using new syntax | for Union and | None for Optional?

I actually saw we have an internal type for this if I'm not mistaken:IndexLabel

@simonjayhawkins would be the person to ask here

@simonjayhawkins any chance you can have a look?

rhshadrach

lgtm, small request

doc/source/whatsnew/v1.2.5.rst

phofl · 2021-04-20T10:25:01Z

doc/source/whatsnew/v1.2.5.rst

@@ -35,7 +35,6 @@ Bug fixes
 Other
 ~~~~~

-


Please remove this

meaning your change, sorry if not clear

jreback · 2021-04-20T12:58:30Z

doc/source/whatsnew/v1.3.0.rst

@@ -219,7 +219,7 @@ Other enhancements
 - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`)
 - :meth:`.GroupBy.any` and :meth:`.GroupBy.all` use Kleene logic with nullable data types (:issue:`37506`)
 - :meth:`.GroupBy.any` and :meth:`.GroupBy.all` return a ``BooleanDtype`` for columns with nullable data types (:issue:`33449`)
-
+- :meth:`DataFrame.dropna` now accepts a single label as ``subset`` along with array-like.


add the issue number (or this PR number if no issue)

pandas/core/frame.py

github-actions · 2021-05-21T00:31:47Z

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.

erfannariman · 2021-10-09T12:33:28Z

@erfannariman closing as stale. ping when ready to continue.

Could you please reopen, will finish this one.

erfannariman · 2021-10-10T13:46:02Z

I get a typing error since subset is IndexLabel, while np.compress expects other type. The functionality should still work, since com.maybe_make_list returns a list.

rhshadrach · 2021-10-10T13:52:11Z

doc/source/whatsnew/v1.3.4.rst

@@ -42,7 +42,7 @@ Bug fixes

 Other
 ~~~~~
-
+- :meth:`DataFrame.dropna` now accepts a single label as ``subset`` along with array-like. (:issue:`41021`)


This should be targeted for 1.4 since it's not a regression. Can go in enhancements there (we try to not use the Other section).

rhshadrach · 2021-10-10T13:58:25Z

pandas/core/frame.py

@@ -5990,6 +5990,8 @@ def dropna(

        agg_obj = self
        if subset is not None:
+            # subset needs to be list
+            subset = com.maybe_make_list(subset)


Looking at the implementation of maybe_make_list, this will only function correctly if the sequence is a list or tuple (all other sequences will be packed as a single element list). Seems to me we either need make the API more restrictive (lists and tuples only), or improve the implementation to handle generic sequences.

e.g.

df = pd.DataFrame({'a': [1, 1, np.nan], 'b': [1, np.nan, 1], 'c': [np.nan, 1, 1]}) print(df.dropna(subset=np.array(['b', 'c'])))

Thanks, reverted back to using is_list_like, which passes the tests. Also added a new test with np.array.

@jreback this means I am not using com.maybe_make_list

pep8speaks · 2021-10-10T14:18:22Z

Hello @erfannariman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-18 14:37:09 UTC

erfannariman · 2021-10-10T19:05:03Z

@rhshadrach @jreback think this one is good to go.

rhshadrach

Logic looks good - just two nit picks and a question.

rhshadrach · 2021-10-13T02:05:13Z

doc/source/whatsnew/v1.4.0.rst

@@ -129,6 +129,7 @@ Other enhancements
 - :meth:`DataFrame.__pos__`, :meth:`DataFrame.__neg__` now retain ``ExtensionDtype`` dtypes (:issue:`43883`)
 - The error raised when an optional dependency can't be imported now includes the original exception, for easier investigation (:issue:`43882`)
 - Added :meth:`.ExponentialMovingWindow.sum` (:issue:`13297`)
+- :meth:`DataFrame.dropna` now accepts a single label as ``subset`` along with array-like. (:issue:`41021`)


No period at the end

pandas/core/frame.py

rhshadrach · 2021-10-16T03:24:00Z

doc/source/whatsnew/v1.3.4.rst

@@ -43,7 +43,6 @@ Bug fixes
 Other
 ~~~~~
 -
-


Can you revert this change

pandas/core/frame.py

jreback

over to you @rhshadrach

rhshadrach

lgtm

rhshadrach · 2021-10-18T15:48:57Z

Thanks @erfannariman!

check for string and convert to list

6afcbcd

erfannariman changed the title ~~check for string and convert to list~~ ENH: check for string and convert to list in DataFrame.dropna subset argument Apr 18, 2021

mzeitlin11 reviewed Apr 18, 2021

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

erfannariman added 5 commits April 18, 2021 21:58

add test for missing key in axis

86d2cd8

add match pytest

03d3e0d

add whatsnew entry

f7a1847

add correct typing and checks

0326c7a

rename typing in docstring:

63a78ef

phofl reviewed Apr 18, 2021

View reviewed changes

doc/source/whatsnew/v1.2.5.rst Outdated Show resolved Hide resolved

phofl reviewed Apr 18, 2021

View reviewed changes

pandas/tests/frame/methods/test_dropna.py Outdated Show resolved Hide resolved

phofl reviewed Apr 18, 2021

View reviewed changes

pandas/tests/frame/methods/test_dropna.py Outdated Show resolved Hide resolved

phofl reviewed Apr 18, 2021

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

erfannariman added 3 commits April 19, 2021 01:12

fix typo whatsnew

93fb0b3

use is list like

103bd35

fix tests

365c6c0

phofl reviewed Apr 19, 2021

View reviewed changes

erfannariman added 2 commits April 19, 2021 14:21

fix typing

cace369

use pandas typing

66d447d

rhshadrach requested changes Apr 20, 2021

View reviewed changes

doc/source/whatsnew/v1.2.5.rst Outdated Show resolved Hide resolved

rhshadrach added the API - Consistency Internal Consistency of API/Behavior label Apr 20, 2021

rhshadrach added this to the 1.3 milestone Apr 20, 2021

erfannariman added 2 commits April 20, 2021 09:56

more clear whatsnew entry

7be5fe7

move whatsnew entry to 1.3

1dd0a64

phofl reviewed Apr 20, 2021

View reviewed changes

revert whatsnew 1.2.5 to master

56ef81e

jreback requested changes Apr 20, 2021

View reviewed changes

github-actions bot added the Stale label May 21, 2021

jreback reopened this Oct 9, 2021

erfannariman added 4 commits October 10, 2021 12:01

merge master and fix conflicts, plus add gh issue to whatsnew

ae633cf

use com.maybe_make_list

4b905e5

remove is list like

c585c8f

remove list like

1d5f208

erfannariman requested review from jreback and rhshadrach October 10, 2021 11:29

rhshadrach requested changes Oct 10, 2021

View reviewed changes

erfannariman added 2 commits October 10, 2021 16:17

move whatsnew entry to 1.4

2bf5ac3

revert to is_list_like and add test with np.array

9ea97ff

erfannariman added 2 commits October 10, 2021 16:19

remove blank line

d09c6ba

use basic indexing instead of np.compress

c09d147

erfannariman requested a review from rhshadrach October 10, 2021 19:03

rhshadrach requested changes Oct 16, 2021

View reviewed changes

jreback requested changes Oct 16, 2021

View reviewed changes

pandas/core/frame.py Show resolved Hide resolved

lithomas1 removed the Stale label Oct 16, 2021

erfannariman added 3 commits October 16, 2021 19:25

merge master

e95200b

remove dot

df5ca75

use list instead of tuple

17fe9bc

jreback approved these changes Oct 16, 2021

View reviewed changes

jreback added this to the 1.4 milestone Oct 16, 2021

rhshadrach mentioned this pull request Oct 17, 2021

CLN: Change implementation of maybe_make_list #44071

Closed

Merge branch 'master' into dropna-accept-string

b471701

rhshadrach approved these changes Oct 18, 2021

View reviewed changes

rhshadrach merged commit 58de58f into pandas-dev:master Oct 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: check for string and convert to list in DataFrame.dropna subset argument #41022

ENH: check for string and convert to list in DataFrame.dropna subset argument #41022

erfannariman commented Apr 18, 2021 •

edited

Loading

erfannariman commented Apr 19, 2021 •

edited

Loading

phofl commented Apr 19, 2021

phofl Apr 19, 2021

erfannariman Apr 19, 2021

phofl Apr 19, 2021

erfannariman Apr 20, 2021

rhshadrach left a comment

phofl Apr 20, 2021

phofl Apr 20, 2021

jreback Apr 20, 2021

github-actions bot commented May 21, 2021

erfannariman commented Oct 9, 2021

erfannariman commented Oct 10, 2021

rhshadrach Oct 10, 2021

rhshadrach Oct 10, 2021

rhshadrach Oct 10, 2021

erfannariman Oct 10, 2021

pep8speaks commented Oct 10, 2021 •

edited

Loading

erfannariman commented Oct 10, 2021

rhshadrach left a comment

rhshadrach Oct 13, 2021

rhshadrach Oct 16, 2021

jreback left a comment

rhshadrach left a comment

rhshadrach commented Oct 18, 2021

@@ @@ -35,7 +35,6 @@ Bug fixes @@
               Other
               ~~~~~
-              -

@@ @@ -43,7 +43,6 @@ Bug fixes @@
               Other
               ~~~~~
               -
-              -

ENH: check for string and convert to list in DataFrame.dropna subset argument #41022

ENH: check for string and convert to list in DataFrame.dropna subset argument #41022

Conversation

erfannariman commented Apr 18, 2021 • edited Loading

erfannariman commented Apr 19, 2021 • edited Loading

phofl commented Apr 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhshadrach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 21, 2021

erfannariman commented Oct 9, 2021

erfannariman commented Oct 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Oct 10, 2021 • edited Loading

Comment last updated at 2021-10-18 14:37:09 UTC

erfannariman commented Oct 10, 2021

rhshadrach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

rhshadrach left a comment

Choose a reason for hiding this comment

rhshadrach commented Oct 18, 2021

erfannariman commented Apr 18, 2021 •

edited

Loading

erfannariman commented Apr 19, 2021 •

edited

Loading

pep8speaks commented Oct 10, 2021 •

edited

Loading