parrt / dtreeviz

A python library for decision tree visualization and model interpretation.
MIT License
2.94k stars 331 forks source link

Development requirement in `setup.py` #314

Closed simonTurintech closed 9 months ago

simonTurintech commented 9 months ago

The current setup.py file requirements includes pytest, which is a development requirement that should not be needed in production (i.e. when users want to install and use the library).

Here's the output of grepping for pytest in the repository:

$ grep 'pytest' **/*.py | uniq
setup.py:        'pytest'
testing/testlib/models/conftest.py:import pytest
testing/testlib/models/conftest.py:@pytest.fixture(autouse=True)
testing/testlib/models/conftest.py:@pytest.fixture()
testing/testlib/models/test_decision_tree_lightgbm_classifier.py:import pytest
testing/testlib/models/test_decision_tree_lightgbm_classifier.py:@pytest.fixture()
testing/testlib/models/test_decision_tree_spark_classifier.py:import pytest
testing/testlib/models/test_decision_tree_spark_classifier.py:@pytest.fixture()
testing/testlib/models/test_decision_trees_sk_classifier.py:import pytest
testing/testlib/models/test_decision_trees_sk_classifier.py:@pytest.fixture()
testing/testlib/models/test_decision_trees_sk_pipeline.py:import pytest
testing/testlib/models/test_decision_trees_sk_pipeline.py:@pytest.fixture()
testing/testlib/models/test_decision_trees_xgb_classifier.py:import pytest
testing/testlib/models/test_decision_trees_xgb_classifier.py:@pytest.fixture()
testing/testlib/models/test_decision_tree_tensorflow_classifier.py:import pytest
testing/testlib/models/test_decision_tree_tensorflow_classifier.py:@pytest.fixture()
testing/testlib/models/test_decision_tree_xgb_regressor.py:import pytest
testing/testlib/models/test_decision_tree_xgb_regressor.py:@pytest.fixture()

This dependency does not seem to be used outside of tests.

Could we consider finding a way to remove it from the mandatory requirements? I suggest creating a new extra, named develop for instance, and adding a short paragraph in the readme about it. I'll open a PR implementing that shortly.

Do not hesitate to get back to me if there's any other reason for pytest to be there, or if you have another idea for fixing this issue :smile:

tlapusan commented 9 months ago

hi @simonTurintech, thanks for creating this issue, your suggestion makes sense for me. I will add few comments on your PR :)