scikit-learn-contrib / py-earth

A Python implementation of Jerome Friedman's Multivariate Adaptive Regression Splines
http://contrib.scikit-learn.org/py-earth/
BSD 3-Clause "New" or "Revised" License
455 stars 122 forks source link

Meet scikit-learn-contrib requirements #96

Closed jcrudy closed 6 years ago

jcrudy commented 8 years ago

I think @mehdidc has already done everything necessary to meet the requirements of contrib. However, @mblondel, could you sign off on this? With confirmation from you, the next priority will be a tagged release and publication on pypi (see issue #76 ).

@mehdidc I'm assigning this issue to you because I think you've basically already solved it. If there's more that needs to be done and you want to assign it back to me, please feel free. Not trying to push more work onto you than you want to take:).

mblondel commented 8 years ago

In lightning, we are using sklearn-contrib-lightning as the package name on pypi https://github.com/scikit-learn-contrib/lightning

It would be nice if you could use the same convention.

mblondel commented 8 years ago

@mblondel, could you sign off on this?

Sure, I will have a look over the week-end.

jcrudy commented 8 years ago

@mblondel Much appreciated. So to follow the convention, our pypi name would be sklearn-contrib-py-earth.

mblondel commented 8 years ago

I got a test failure.

======================================================================
ERROR: pyearth.test.test_earth.test_untrained
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mblondel/anaconda/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/mblondel/Desktop/projects/py-earth/pyearth/test/test_earth.py", line 436, in test_untrained
    assert_raises(NotFittedError, model.predict, X)
  File "/Users/mblondel/anaconda/lib/python2.7/unittest/case.py", line 473, in assertRaises
    callableObj(*args, **kwargs)
  File "/Users/mblondel/Desktop/projects/py-earth/pyearth/earth.py", line 851, in predict
    B = self.transform(X, missing)
  File "/Users/mblondel/Desktop/projects/py-earth/pyearth/earth.py", line 1004, in transform
    check_is_fitted(self, "basis_")
  File "/Users/mblondel/Desktop/projects/scikit-learn/sklearn/utils/validation.py", line 685, in check_is_fitted
    raise _NotFittedError(msg % {'name': type(estimator).__name__})
NotFittedError: This Earth instance is not fitted yet. Call 'fit' with appropriate arguments before using this method.

----------------------------------------------------------------------

FYI, I am using scikit-learn master.

mblondel commented 8 years ago

This is very hackish: https://github.com/scikit-learn-contrib/py-earth/blob/master/pyearth/test/test_earth.py#L45

We need to support this at the API level. CC @amueller

mblondel commented 8 years ago

You might want to add appveyor for testing py-earth on Windows. Here are the files used in scikit-learn and lightning: https://github.com/scikit-learn/scikit-learn/blob/master/appveyor.yml https://github.com/scikit-learn-contrib/lightning/blob/master/appveyor.yml

amueller commented 8 years ago

I know. The current "estimator_type" is a bad idea and should be a dict "estimator_tags".

jcrudy commented 8 years ago

Thanks, @mblondel. I'll get to work on these notes and see if I can reproduce the test failure.

jcrudy commented 6 years ago

Version 0.1.0 is now on pypi.