search

scikit-learn-contrib / skope-rules

machine learning with logical rules in Python
http://skope-rules.readthedocs.io
Other
599 stars 96 forks source link

Remove unnecessary checks for numpy/scipy in setup.py. #24

Closed timstaley closed 4 years ago

timstaley commented 5 years ago

These deps are listing in install_requires, so will get installed by pip.

Addresses issue #19.

Interesting project, thanks for your dev efforts folks :-)

timstaley commented 5 years ago

Hmm. Travis CI is failing with the following test-fail, which at first glance doesn't look to be build-related but might be a scikit-learn version incompatibility I guess?

FAIL: skrules.tests.test_common.test_classifier
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/testenv/lib/python3.6/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/build/scikit-learn-contrib/skope-rules/skrules/tests/test_common.py", line 7, in test_classifier
    return check_estimator(SkopeRules)
  File "/home/travis/miniconda/envs/testenv/lib/python3.6/site-packages/scikit_learn-0.20.3-py3.6-linux-x86_64.egg/sklearn/utils/estimator_checks.py", line 296, in check_estimator
    check_no_attributes_set_in_init(name, estimator)
  File "/home/travis/miniconda/envs/testenv/lib/python3.6/site-packages/scikit_learn-0.20.3-py3.6-linux-x86_64.egg/sklearn/utils/testing.py", line 348, in wrapper
    return fn(*args, **kwargs)
  File "/home/travis/miniconda/envs/testenv/lib/python3.6/site-packages/scikit_learn-0.20.3-py3.6-linux-x86_64.egg/sklearn/utils/estimator_checks.py", line 1996, in check_no_attributes_set_in_init
    % (name, sorted(invalid_attr)))
AssertionError: {'max_depths'} is not false : Estimator SkopeRules should not set any attribute apart from parameters during init. Found attributes ['max_depths'].
timstaley commented 5 years ago

Right, I've got the tests passing locally. Looks like scikit-learn 0.20 implements some stricter checks in the generic check_estimator which aren't currently satisfied by skope-rules, I'll leave that to you core devs to figure out for now!

ngoix commented 5 years ago

can you please review #23 to speed up the process?

jamesmyatt commented 5 years ago

Is there anything I can do to help get this merged?

AlJohri commented 4 years ago

@ngoix @timstaley would love to get this merged soon. thanks!

ecederstrand commented 4 years ago

It would be really nice to have this fixed. It's an annoyance that you can't just put skope-rules in a requirements.txt and expect it to work.

ecederstrand commented 4 years ago

@ngoix Thanks for merging this.

Would it be possible for you to release a version to pypi.org containing this patch?