MNT Use raise from in 19 modules

[cool-RR]

@NicolasHug This is the continuation of #17545

After we're done with this PR, I'll do another PR to fix the rest.

[NicolasHug]

thanks @cool-RR , a few comments

[cmarmo]

Hi @cool-RR codecov/patch check is unhappy: clicking on the Details link will give you all the information.

For example, for one of the files sklearn/inspection/_partial_dependence.py you can check here the lines of the code that are not tested in red.
A specific test should be added in those cases.
Thanks!

[cool-RR]

@cmarmo Ah... We didn't do any of that testing in the previous PRs in this series. I'm a little reluctant to dig into scikit-learn's test architecture, because I went through a lot of files making the same change. Any chance I could get a pass on this?

[cmarmo]

I haven't check in details where the codecov report has failed. A nice summary tells you which files are affected in your pull request.
I didn't know how to write tests in scikit-learn until last year too, don't let you intimidate... :)
You can check the documentation, but as an example for _partial_dependence.py you should add a test in sklearn/inspection/tests/test_partial_dependence.py.

• First you create a simple code reproducing the error that should be tested
• then you verify that it is thrown using pytest, like here

Additional resources about tests are also available here.

Any chance I could get a pass on this?

I'm not the one deciding, sorry .. :).

[cool-RR]

@NicolasHug Because I haven't gotten a reply on this PR for a long time, it now has a conflict. I can solve the merge conflict, but I first want to know whether you are even interested in accepted this PR, and whether there are any problems besides the merge conflict that would prevent this PR from being merged.

[thomasjpfan]

I would be +1 for merging this.

[cool-RR]

@thomasjpfan I'm seeing the CI is failing with this line: INTERNALERROR> AttributeError: 'WorkerController' object has no attribute 'slaveinput'

This doesn't seem related to my changes. Any idea why it's failing? How can I fix this?

[thomasjpfan]

This should have been fixed with pytest-dev/pytest-xdist#287

It could be an issue with pytest 6.0 I'll look into it.

[cool-RR]

So do we rerun? Do I need to push a new commit for that?

…

On Fri, Aug 14, 2020, 19:08 Thomas J. Fan ***@***.***> wrote: This should have been fixed with pytest-dev/pytest-xdist#287 <pytest-dev/pytest-xdist#287> It could be an issue with pytest 6.0 I'll look into it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17835 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAN3SRW5UULPXEIHWXCRX3SAVOPVANCNFSM4OQYR2EA> .

[thomasjpfan]

The issue will resolve with pytest-dev/pytest-cov#411 when pytest-cov==2.10.1 gets released. It was caused by pytest-xdist updating to 2.0.

There is no action we can take in this PR for now.

I have a temporary fix at #17835

[cool-RR]

@thomasjpfan I assume that the codecov/patch is something I can ignore, right? If so, are we good for merging this PR?

[thomasjpfan]

As for codecov, this means that some of these exceptions are not tested. I think I am okay with ignoring them in this PR.

[cool-RR]

@thomasjpfan Anything else needed before merging?

[rth]

Thanks @cool-RR !