stanfordmlgroup / ngboost

Natural Gradient Boosting for Probabilistic Prediction
Apache License 2.0
1.64k stars 213 forks source link

Implementation of unit testing #41

Open yutayamazaki opened 4 years ago

yutayamazaki commented 4 years ago
avati commented 4 years ago

@yutayamazaki could you describe what error you got with the two classes?

tonyduan commented 4 years ago

I'm able to reproduce this issue by attempting to run the file [1]. The issue seems to arise due to the way Scikit-Learn is using GridSearchCV; any BaseEstimator sub-class must be explicit about its parameters and specifically should not take *kwargs or args.

My two cents on the Scikit-Learn API integration:

Edit: this has been implemented and should be working now, see examples/sklearn_cv.py.

[1] https://github.com/stanfordmlgroup/ngboost/blob/master/examples/empirical/clf_sklearn.py

wakame1367 commented 4 years ago

35

alejandroschuler commented 4 years ago

note to get CI up and running when we have some better tests: https://github.com/stanfordmlgroup/ngboost/issues/36

alejandroschuler commented 4 years ago

I also want to try out pytest instead of unittest- seems cleaner.

alejandroschuler commented 4 years ago

note to add tests for every combination of distribution and scoring rule

alejandroschuler commented 4 years ago

note to add tests for sample weighting

wakame1367 commented 4 years ago

I also want to try out pytest instead of unittest- seems cleaner.

Should I rewrite tests from unittest to tests using pytest?

alejandroschuler commented 4 years ago

@wakamezake sure, it would be welcome! I don't have any experience with either but since we're just getting started it's a good time to pick a framework and pytest seems a little easier.

alejandroschuler commented 4 years ago

hey @wakamezake I've started to expand the test suite a bit and I'm wondering if you're interested in lending a hand. Ideally I'd like to test all the combinations of all of the features that we've laid out in the vignette in a way that's similar to how you wrote the tests in test_basic.py and what I have now in test_distns.py, although in a way that's modular and doesn't require a lot of code copy-pasting. Ideally we would also unit test some of the low-level features like what's described in the developer guide and even the low-level helper methods in ngboost.py. At a first pass, though, ensuring that all of the high-level features work with each other without throwing errors would be good enough.

Is that something you (or anyone else here) would be interested in working on?

Thanks!

alejandroschuler commented 4 years ago

https://hypothesis.readthedocs.io/en/latest/quickstart.html

ryan-wolbeck commented 4 years ago

hey @wakamezake I've started to expand the test suite a bit and I'm wondering if you're interested in lending a hand. Ideally I'd like to test all the combinations of all of the features that we've laid out in the vignette in a way that's similar to how you wrote the tests in test_basic.py and what I have now in test_distns.py, although in a way that's modular and doesn't require a lot of code copy-pasting. Ideally we would also unit test some of the low-level features like what's described in the developer guide and even the low-level helper methods in ngboost.py. At a first pass, though, ensuring that all of the high-level features work with each other without throwing errors would be good enough.

Is that something you (or anyone else here) would be interested in working on?

Thanks!

Hey @alejandroschuler did these links move somewhere else?

alejandroschuler commented 4 years ago

@ryan-wolbeck good question- both the vignette and developer guide were rolled into the one-stop-shop user guide, so neither exist anymore.