Skip to content

ENH: Show column name in assert_frame_equal #29218

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

Jump to bottom
Merged
merged 14 commits into from
Jan 7, 2020
Merged

ENH: Show column name in assert_frame_equal #29218

merged 14 commits into from
Jan 7, 2020

Conversation

moi90
Copy link
Contributor

@moi90 moi90 commented Oct 25, 2019 •
edited
Loading

The output of the AssertionError raised by assert_frame_equal is changed:

  • old: AssertionError: Attributes of DataFrame.iloc[:, 1] are different

  • new: AssertionError: Attributes of DataFrame.loc[:, 'b'] are different

  • closes #xxxx

  • tests added / passed

  • passes black pandas

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

  • whatsnew entry

@moi90
ENH: Show column name in assert_frame_equal
be0e355 
The output of the AssertionError is changed:

- old: `AssertionError: Attributes of DataFrame.iloc[:, 1] are different`
- new: `AssertionError: Attributes of DataFrame.loc[:, 'b'] are different`
@jreback
Copy link
Contributor

jreback commented Oct 25, 2019

hmm, the only issue is this doesn't quite work for duplicated names, but i guess this is friendlier for readers of the error message.

@jreback
Copy link
Contributor

jreback commented Oct 25, 2019

cc @TomAugspurger @jorisvandenbossche

@jreback jreback added the Compat pandas objects compatability with Numpy or Python functions label Oct 25, 2019
@TomAugspurger
Copy link
Contributor

Yeah duplicates labels was my initial concern.

Can we update the error message to keep using .iloc[:, i], but also include column name=" " somewhere?

@moi90
Copy link
Contributor Author

moi90 commented Oct 25, 2019

Thanks for the feedback!
I see, I didn't think about duplicate names (and didn't know that they are allowed at all).

@pep8speaks
Copy link

pep8speaks commented Oct 25, 2019 •
edited
Loading

Hello @moi90! 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 2020-01-06 20:45:56 UTC

@moi90
Copy link
Contributor Author

moi90 commented Oct 25, 2019

Sorry for the multiple commits. Feel free to squash.

@jbrockmendel
Copy link
Member

@moi90 no need to worry about the squashing, but we do need to get the tests passing. looks like there are a handful of places where you need to update the expected error message

@moi90
Copy link
Contributor Author

moi90 commented Nov 1, 2019

I will look into this.

@alimcmaster1
Copy link
Member

@moi90 any luck? If you have questions feel free to post them on there :)

WillAyd
WillAyd reviewed Nov 19, 2019
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Looks good to me. Optional comment from my perspective

pandas/util/testing.py Outdated
@@ -1400,7 +1400,9 @@ def assert_frame_equal(
check_names=check_names,
check_datetimelike_compat=check_datetimelike_compat,
check_categorical=check_categorical,
obj="{obj}.iloc[:, {idx}]".format(obj=obj, idx=i),
obj='{obj}.iloc[:, {idx}] (column name="{col}")'.format(
Copy link
Member

Choose a reason for hiding this comment

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

If you wanted to could just use f-strings to make this even more compact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are new in 3.6. Is anything older not supported anymore by pandas?

Copy link
Contributor

Choose a reason for hiding this comment

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

no we just dropped 3.5

@jreback jreback added this to the 1.0 milestone Nov 19, 2019
@jreback
Copy link
Contributor

jreback commented Nov 19, 2019

@moi90 if you can use f-strings and ping on green.

@jbrockmendel
Copy link
Member

@moi90 can you rebase

WillAyd
WillAyd requested changes Dec 9, 2019
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

This looks close. Looks like maybe an issue on previous conflict fix ups; if you can resolve and repush would be great!

pandas/util/testing.py Outdated

return wrapper


with_connectivity_check = network


def assert_raises_regex(_exception, _regexp, _callable=None, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

This was removed in #29174 so want to be careful not to add back in here

@jbrockmendel
Copy link
Member

the CI error in the linter usually means there is an unused import somehwere

@moi90
Copy link
Contributor Author

moi90 commented Dec 29, 2019

No, it's an error in flake8:

   File "/home/runner/miniconda3/envs/pandas-dev/lib/python3.7/site-packages/flake8/formatting/default.py", line 39, in format
    "col": error.column_number,
ValueError: unsupported format character ':' (0x3a) at index 41

I'm beginning to give up... Can't someone of the core team bring it to a successful conclusion?

@jbrockmendel
Copy link
Member 

ValueError: unsupported format character ':' (0x3a) at index 41

The last few times I've seen this, it turned out there was an unused import somewhere.

@jreback jreback removed this from the 1.0 milestone Jan 1, 2020
@jreback
Copy link
Contributor

jreback commented Jan 1, 2020

can you merge master

@jorisvandenbossche
Copy link
Member

@moi90 I merged master and fixed some flake8 issues. Not sure why it failed before, but know that you can always run the linting checks locally (I recommend installing git commit hooks for this: https://dev.pandas.io/docs/development/contributing.html#python-pep8-black).

All green now, so will merge. Thanks for the PR!

@jorisvandenbossche jorisvandenbossche merged commit 090957c into pandas-dev:master Jan 7, 2020
@jorisvandenbossche jorisvandenbossche added this to the 1.0 milestone Jan 7, 2020
@moi90
Copy link
Contributor Author

moi90 commented Jan 9, 2020

Wonderful, thanks! And you're welcome :)

@moi90 moi90 deleted the assert_frame_equal-colname branch January 9, 2020 12:59
@amilbourne amilbourne mentioned this pull request Jan 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

@WillAyd WillAyd WillAyd requested changes

Assignees
No one assigned
Labels
Compat pandas objects compatability with Numpy or Python functions
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
8 participants
@moi90 @jreback @TomAugspurger @pep8speaks @jbrockmendel @alimcmaster1 @jorisvandenbossche @WillAyd