scikit-learn-contrib / scikit-learn-contrib

scikit-learn compatible projects
407 stars 50 forks source link

Request to add t-Student-Mixture-Models #18

Open luiscarlosgph opened 7 years ago

luiscarlosgph commented 7 years ago

Request for project inclusion in scikit-learn-contrib

GaelVaroquaux commented 7 years ago

I don't think that you have continuous integration on travis (or circle). We are not requiring it for integration in scikit-learn-contrib (I think that we should, @mblondel, @amueller, any opinion), but I think that it would be much better for your project.

vene commented 7 years ago

I'm +1 for adding a requirement for CI. (maybe at least a soft requirement.) It's not so difficult to set up as long as the test suite already exists, and we do require tests.

I had subtle bugs in polylearn because of using non-initialized memory in cython which only showed up on 32-bit windows thanks to Appveyor. Would never have caught them without CI & thorough tests.

luiscarlosgph commented 7 years ago

Thanks for the quick feedback, I will add the CI with Travis now and come back to you when done.

amueller commented 7 years ago

It's the last point on the the workflow document: image

Where did you get from that we don't require it? If that requirement (and I think it is a requirement) is not documented in a particular place, we need to add it.

luiscarlosgph commented 7 years ago

CI with Travis: https://travis-ci.org/luiscarlosgph/t-Student-Mixture-Models

GaelVaroquaux commented 7 years ago

Where did you get from that we don't require it?

OK. I guess we are all on the page. I was maybe trying to be too careful/polite.

GaelVaroquaux commented 7 years ago

With regards to integrating this, it's a pity that the docs are not visible online. Is there a plan to change this after integration?

luiscarlosgph commented 7 years ago

I have put the documentation available here: http://t-student-mixture-models.readthedocs.io/en/latest