Skip to content

BUG: Respect check_dtype in assert_series_equal #32757

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 16 commits into from
Mar 17, 2020
Merged

BUG: Respect check_dtype in assert_series_equal #32757

merged 16 commits into from
Mar 17, 2020

Conversation

dsaxton
Copy link
Member

@dsaxton dsaxton commented Mar 16, 2020 •
edited
Loading

I'm not sure if https://github.com/pandas-dev/pandas/blob/master/pandas/_testing.py#L1174 needs to change too?

cc @jorisvandenbossche

jorisvandenbossche
jorisvandenbossche reviewed Mar 16, 2020
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche 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 looking into this!

Can you also add a test for the assert_frame_equal case? (so to ensure it is properly passed through)

pandas/tests/util/test_assert_series_equal.py Outdated
with pytest.raises(AssertionError, match=msg):
tm.assert_series_equal(left, right, check_dtype=check_dtype)
else:
tm.assert_series_equal(left, right, check_dtype=check_dtype)
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 just test both True / False cases inline here without the parametrization? (it actually gets you rid of the if/else, while for the rest not adding any code)

Copy link
Member

Choose a reason for hiding this comment

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

(and same below)

@pep8speaks
Copy link

pep8speaks commented Mar 16, 2020 •
edited
Loading

Hello @dsaxton! 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-03-17 18:01:47 UTC

@jreback jreback added Bug Testing pandas testing functions or related to the test suite labels Mar 17, 2020
@jreback jreback added this to the 1.1 milestone Mar 17, 2020
@jreback
Copy link
Contributor

jreback commented Mar 17, 2020

can you add a whatsnew note (can put in bug fixes other section)

jorisvandenbossche
jorisvandenbossche reviewed Mar 17, 2020
View reviewed changes
@simonjayhawkins
Copy link
Member

can you add a whatsnew note (can put in bug fixes other section)

IIUC the bug is only on master, not released. Is the whatsnew needed?

@jorisvandenbossche
Copy link
Member

IIUC the bug is only on master, not released. Is the whatsnew needed?

Part of the fix did fix something compared to 1.0 (eg for intervals)

@jorisvandenbossche jorisvandenbossche merged commit 05780a7 into pandas-dev:master Mar 17, 2020
@jorisvandenbossche
Copy link
Member

Thanks @dsaxton !

@dsaxton dsaxton deleted the assert-series branch March 17, 2020 19:35
@simonjayhawkins
Copy link
Member

Part of the fix did fix something compared to 1.0 (eg for intervals)

If this fixes something that worked in 1.0, then should this be backported?

@jorisvandenbossche
Copy link
Member

Sorry, I meant: it's partly fixing a bug that is in 1.0 (hence the whatsnew note is fine), and partly a regression that is only in master (which doesn't need a whatsnew).
But the bug from 1.0 is not a regression nor critical, so shouldn't need a backport

@simonjayhawkins
Copy link
Member

ah. my bad.

SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020
@dsaxton @SeeminSyed
BUG: Respect check_dtype in assert_series_equal (pandas-dev#32757)
ebab47b
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Mar 23, 2020
@dsaxton @jbrockmendel
BUG: Respect check_dtype in assert_series_equal (pandas-dev#32757)
00045aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

Assignees
No one assigned
Labels
Bug Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

BUG: Regression in assert_frame_equal check_dtype for extension dtypes
5 participants
@dsaxton @pep8speaks @jreback @simonjayhawkins @jorisvandenbossche