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

FIX-#4409: Fix eval_insert utility that doesn't actually check results of insert function #4410

Merged
merged 6 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/release_notes/release_notes-0.15.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Key Features and Updates
* FIX-#4422: get rid of case sensitivity for `warns_that_defaulting_to_pandas` (#4423)
* TEST-#4426: Stop passing is_default kwarg to Modin and pandas (#4428)
* FIX-#4439: Fix flake8 CI fail (#4440)
* FIX-#4409: Fix `eval_insert` utility that doesn't actually check results of `insert` function (#4410)
* Documentation improvements
* DOCS-#4296: Fix docs warnings (#4297)
* DOCS-#4388: Turn off fail_on_warning option for docs build (#4389)
Expand Down
6 changes: 5 additions & 1 deletion modin/pandas/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,11 @@ def insert(self, loc, column, value, allow_duplicates=False): # noqa: PR01, D20
and not isinstance(value, (pandas.Series, Series))
and len(value) != len(self.index)
):
raise ValueError("Length of values does not match length of index")
raise ValueError(
"Length of values ({}) does not match length of index ({})".format(
len(value), len(self.index)
)
)
Comment on lines +1164 to +1168
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the process of manual testing, I accidentally found a message mismatch in an exception regarding pandas.

if not allow_duplicates and column in self.columns:
raise ValueError(f"cannot insert {column}, already exists")
if not -len(self.columns) <= loc <= len(self.columns):
Expand Down
2 changes: 1 addition & 1 deletion modin/pandas/test/dataframe/test_map_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def eval_insert(modin_df, pandas_df, **kwargs):
modin_df,
pandas_df,
operation=lambda df, **kwargs: df.insert(**kwargs),
__inplace__=True,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Insert function doesn't return anything, so we should explicitly use __inplace__=True here.

**_kwargs,
)

Expand Down Expand Up @@ -1049,7 +1050,6 @@ def comparator(df1, df2):
loc=0,
col=f"test_col{idx}",
value=value,
__inplace__=True,
comparator=lambda df1, df2: comparator(df1, df2),
)

Expand Down