Skip to content

TST: disallow bare pytest raises #34940

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 5 commits into from
Jun 23, 2020
Merged

TST: disallow bare pytest raises #34940

merged 5 commits into from
Jun 23, 2020

Conversation

boweyism
Copy link
Contributor

@boweyism boweyism commented Jun 22, 2020 •
edited by jreback
Loading

xref #30999

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

Changes for:

  • pandas/tests/frame/methods/test_assign.py
  • pandas/tests/frame/methods/test_at_time.py
  • pandas/tests/frame/methods/test_between_time.py
  • pandas/tests/frame/methods/test_first_and_last.py
  • pandas/tests/frame/methods/test_interpolate.py
  • pandas/tests/frame/methods/test_replace.py
  • pandas/tests/frame/test_query_eval.py

Belongs to #30999

simonjayhawkins
simonjayhawkins reviewed Jun 23, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @boweyism for the PR. generally lgtm.

pandas/tests/frame/test_query_eval.py Outdated
@@ -870,7 +880,7 @@ def test_str_query_method(self, parser, engine):

for lhs, op, rhs in zip(lhs, ops, rhs):
ex = f"{lhs} {op} {rhs}"
msg = r"'(Not)?In' nodes are not implemented"
# msg = r"'(Not)?In' nodes are not implemented"
Copy link
Member

Choose a reason for hiding this comment

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

don't leave this commented out. out of curiosity why move outside if block if message repeated on L911?

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 left that in there by mistake. Also, I overlooked the if block. I'll put it there. (I had originally put it inside of the for loop)

Copy link
Member

@simonjayhawkins simonjayhawkins Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

can you remove this line entirely. we don't want commented out code left behind.

Copy link
Contributor Author

@boweyism boweyism Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I removed it:
336c96d

Copy link
Member

Choose a reason for hiding this comment

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

still showing in diff. we don't want commented out code left behind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops.. my bad. test_str_query_method and test_str_LIST_query_method got me confused. I've updated it

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Jun 23, 2020
@simonjayhawkins simonjayhawkins added Clean Testing pandas testing functions or related to the test suite labels Jun 23, 2020
@jreback jreback changed the title Tst frame changes for 30999 TST: disallow bare pytest raises Jun 23, 2020
simonjayhawkins
simonjayhawkins approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @boweyism for the changes. lgtm pending green.

@jreback jreback merged commit e4a553a into pandas-dev:master Jun 23, 2020
@jreback
Copy link
Contributor

jreback commented Jun 23, 2020

thanks @boweyism

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonjayhawkins simonjayhawkins simonjayhawkins approved these changes

Assignees
No one assigned
Labels
Clean 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.
3 participants
@boweyism @jreback @simonjayhawkins