Skip to content

Pandas get_dummies validate columns input #28463

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Oct 22, 2019
Merged

Pandas get_dummies validate columns input #28463

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
87fa4ea
Updated Reshape.py to validate `columns` in `get_dummies`
R1j1t Sep 16, 2019
09a4f5e
added tests for the column validation
R1j1t Sep 16, 2019
ef2d577
updated test_reshape.py with PEP-8 code rec
R1j1t Sep 16, 2019
faa8f07
code formated using black
R1j1t Sep 16, 2019
81c2597
updated test_reshape.py
R1j1t Sep 17, 2019
3aa8749
updated file based on the changes in PR
R1j1t Sep 17, 2019
af99037
Update doc/source/whatsnew/v1.0.0.rst
R1j1t Sep 17, 2019
2353462
updated error message based on comments received
R1j1t Sep 17, 2019
7d68181
updated the test with the new error message
R1j1t Sep 17, 2019
ef1b9cd
Removed list-like object check
R1j1t Sep 20, 2019
38f13f9
Merge remote-tracking branch 'upstream/master' into R1j1t-master
TomAugspurger Sep 23, 2019
4da4149
whitespace fixup
WillAyd Sep 23, 2019
aea5019
Merge remote-tracking branch 'upstream/master' into R1j1t-master
WillAyd Oct 11, 2019
9cd8d40
Removed trailing space
R1j1t Oct 12, 2019
f266f47
Removed redundant value in test
R1j1t Oct 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v1.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ Reshaping
^^^^^^^^^

- Bug in :meth:`DataFrame.stack` not handling non-unique indexes correctly when creating MultiIndex (:issue: `28301`)
-
- Added test to assert the :func:`get_dummies` raises TypeError message when the value to `columns` isn't a list-like value (:issue:`28383`)
-

Sparse
^^^^^^
Expand Down
2 changes: 2 additions & 0 deletions pandas/core/reshape/reshape.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,8 @@ def get_dummies(
# determine columns being encoded
if columns is None:
data_to_encode = data.select_dtypes(include=dtypes_to_encode)
elif not is_list_like(columns):
raise TypeError("Input must be a list-like of list-likes")
Copy link
Contributor

Choose a reason for hiding this comment

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

The message should state which parameter is incorrect (columns?)

And is it supposed to be a list-like of list-likes? This block is just checking that it's a sequence, but not making any assertion about what each element is.

Copy link
Contributor Author

@R1j1t R1j1t Sep 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The message should state which parameter is incorrect

I have updated the Error message based on your comment but let me point out this code which was the reason I did not update the error message.

making any assertion about what each element is.

I think this is out of scope because for all the functions definitions (involving list-like object as parameters ) I referred to in pandas, none of them had this check. @jbrockmendel Is this something which should have been checked?

else:
data_to_encode = data[columns]

Expand Down
63 changes: 63 additions & 0 deletions pandas/tests/reshape/test_reshape.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,69 @@ def test_get_dummies_all_sparse(self):
)
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize(
"values",
[
["baz", "zoo"],
np.array(["baz", "zoo"]),
pd.Series(["baz", "zoo"]),
pd.Index(["baz", "zoo"]),
],
)
def test_get_dummies_with_list_like_values(self, values):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this testing new behavior? Are we not already testing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the file reshape.py there was no check on dtype passed to one of the parameters columns and so on the issue it was mentioned add this check. Hence, after adding the test I added these tests specifically to columns parameter. And regarding doubt whether this test was already present, I checked all tests again and there was no test to check the inputs to parameter columns and so I referred to a similar test as mentioned in PR and added it. Please let me know if you spotted a similar test for get_dummies which I might have missed. I hope this clears it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like

pandas/pandas/tests/reshape/test_reshape.py

Line 90 in 367670e

result = get_dummies(s_df, columns=s_df.columns, sparse=sparse, dtype=dtype)
tests with both columns and dtype?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line you are have pointed to does not check for string inputs to the columns parameters also it does not test for other list-like objects passed to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this test seems superfluous; this PR should just check for invalid values in columns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this test. It would be great help if could you elaborate/point me to a similar test for my reference? Because as far i understand if the value is not list-like the error message will be raised because of this PR and if invalid value (i.e. value not in data columns) is passed to columns then error will be raised because value will not be present.

Copy link
Contributor

Choose a reason for hiding this comment

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

if the value is not list-like the error message will be raised because of this PR and if invalid value (i.e. value not in data columns) is passed to columns then error will be raised because value will not be present.

This test is redundant with

pandas/pandas/tests/reshape/test_reshape.py

Line 90 in 367670e

result = get_dummies(s_df, columns=s_df.columns, sparse=sparse, dtype=dtype)
.

We want to keep your test test_get_dummies_with_string_values, which is testing the code you've added in this PR.

# issue #28383
df = pd.DataFrame(
{
"bar": [1, 2, 3, 4, 5, 6],
"foo": ["one", "one", "one", "two", "two", "two"],
"baz": ["A", "B", "C", "A", "B", "C"],
"zoo": ["x", "y", "z", "q", "w", "t"],
}
)

result = pd.get_dummies(df, columns=values, dtype="int64")

data = [
[1, "one", 1, 0, 0, 0, 0, 0, 1, 0, 0],
[2, "one", 0, 1, 0, 0, 0, 0, 0, 1, 0],
[3, "one", 0, 0, 1, 0, 0, 0, 0, 0, 1],
[4, "two", 1, 0, 0, 1, 0, 0, 0, 0, 0],
[5, "two", 0, 1, 0, 0, 0, 1, 0, 0, 0],
[6, "two", 0, 0, 1, 0, 1, 0, 0, 0, 0],
]
columns = [
"bar",
"foo",
"baz_A",
"baz_B",
"baz_C",
"zoo_q",
"zoo_t",
"zoo_w",
"zoo_x",
"zoo_y",
"zoo_z",
]
expected = DataFrame(data=data, columns=columns)
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize("values", ["baz", "zoo"])
Copy link
Member

Choose a reason for hiding this comment

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

are these two values testing meaningfully distinct cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought of adding 2 different values just for the check but they are testing the same case. I will remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only one now!

def test_get_dummies_with_string_values(self, values):
# issue #28383
df = pd.DataFrame(
{
"bar": [1, 2, 3, 4, 5, 6],
"foo": ["one", "one", "one", "two", "two", "two"],
"baz": ["A", "B", "C", "A", "B", "C"],
"zoo": ["x", "y", "z", "q", "w", "t"],
}
)

msg = "Input must be a list-like of list-likes"

with pytest.raises(TypeError, match=msg):
pd.get_dummies(df, columns=values)


class TestCategoricalReshape:
def test_reshaping_multi_index_categorical(self):
Expand Down