Skip to content

Conversation

@ShaharNaveh
Copy link
Contributor

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

After #31130 got merged, I can do what I couldn't do here

@ShaharNaveh ShaharNaveh added the Testing pandas testing functions or related to the test suite label Feb 7, 2020
@WillAyd
Copy link
Member

WillAyd commented Feb 8, 2020

I think this is confusing to say external_error_raised for something in internals

@WillAyd
Copy link
Member

WillAyd commented Feb 8, 2020

Can you alternately not just add the message?

@ShaharNaveh
Copy link
Contributor Author

I think this is confusing to say external_error_raised for something in internals

@WillAyd I am with you on that, still it's raising an external error message, do you suggest moving it to somewhere else?

@ShaharNaveh
Copy link
Contributor Author

Can you alternately not just add the message?

@WillAyd I really prefer not to, in #30999 we are fixing all the bare pytest raises, and in order to silent the test case in #30755 we have to pass in the "match" argument, we could just pass in the match=None but as you said here this is not very clear.

@WillAyd
Copy link
Member

WillAyd commented Feb 12, 2020

I don't have a problem with match=None per se - where you linked my comment I was just asking what the purpose of it was

As mentioned I don't think this adds any value and if anything is confusing, but let's see what others think

@WillAyd
Copy link
Member

WillAyd commented Feb 20, 2020

Closing as I don't think this is moving in the right direction, but thanks in any case @MomIsBestFriend !

@WillAyd WillAyd closed this Feb 20, 2020
@ShaharNaveh ShaharNaveh deleted the TST-fix-external-errors branch February 29, 2020 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing pandas testing functions or related to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants