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

TST: GH30999 Add match=msg to all "with pytest.raises" in pandas/tests/io/pytables/test_store.py #38609

Conversation

moink
Copy link
Member

@moink moink commented Dec 21, 2020

This pull request xref #30999 to remove bare pytest.raises. It doesn't close that issue as I have only addressed one file: pandas/tests/io/pytables/test_store.py. In that file there were 80 instances of bare pytest.raises.

test_append_to_multiple_dropna_false is marked as failing because it isn't raising the ValueError that it should. Because of this, I didn't know what the message should be, and reading through the code didn't help me figure it out. I wrote a draft error message and added a TODO (line 3790) and I hope that's the best way to handle it.

In test_multiple_open_close there were several assertions that ClosedFileError is raised with the same error message, with one in the middle that asserts that an AttributeError is raised with a different error message. I moved the one for the AttributeError to the end of the list (line 4229) to organize the assertions a little better and make clear that the msg parameter is the same for all the ClosedFileErrors.

I did not add a whatsnew entry since it only changes the tests. Let me know if I should add one (and I am a bit unclear on how, i.e. what version this would end up in).

@jreback jreback added Error Reporting Incorrect or improved errors from pandas IO HDF5 read_hdf, HDFStore labels Dec 21, 2020
@moink
Copy link
Member Author

moink commented Dec 21, 2020

I can't figure out what went wrong with the Travis CI build. Any hints from anyone would be appreciated.

@jreback
Copy link
Contributor

jreback commented Dec 21, 2020

I can't figure out what went wrong with the Travis CI build. Any hints from anyone would be appreciated.

ignore it, giving us trouble.

@jreback jreback added this to the 1.3 milestone Dec 21, 2020
@jreback
Copy link
Contributor

jreback commented Dec 21, 2020

@moink can you merge master and ping on green (or greenish if the travis build fails like that again).

@jreback
Copy link
Contributor

jreback commented Dec 21, 2020

OT: @moink love to split this file up: pandas/tests/io/pytables/test_store.py in a separate PR :->

@jreback jreback merged commit 7210b8b into pandas-dev:master Dec 22, 2020
@jreback
Copy link
Contributor

jreback commented Dec 22, 2020

thanks @moink very nice

@moink
Copy link
Member Author

moink commented Dec 24, 2020

OT: @moink love to split this file up: pandas/tests/io/pytables/test_store.py in a separate PR :->

I agree - I noted that when I made these changes I pushed test_store.py over 5000 lines. But I would note that the module it is testing is over 5200 lines and in my opinion also needs to be split up, and then the test module perhaps split up in the same way as the code module.

I am not actually knowledgeable about PyTables and I suspect wouldn't be the best person to do this task. Would you like me to create a new issue?

@jreback
Copy link
Contributor

jreback commented Dec 24, 2020

you can create a new issue

but splitting the tests first is much easier just for visibility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas IO HDF5 read_hdf, HDFStore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants