-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Document matplotlib dependency/ install matplotlib in CI #13335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@qinhanmin2014 do you want just the modules/tree.rst updated to include this dependency or is there other documents that need this added as well? |
I can't see anywhere where we obviously state the requirements for running
examples, which should also include versions. I vote for including it in
doc/install.rst
|
@jnothman readme? |
I only suggest install.rst because other dependency versions are listed
there
|
I mean we obviously state the requirements for running examples in README.rst and I think that's enough. If you want to copy it to install.rst, let's do it. |
Sorry, didn't look properly at README. Let's do it there, then.
|
Ok. Will do it later tonight. |
@maftabali can we pass this onto someone else? |
@jnothman sorry, been snowed under. Do you want this just in the readme or in the install.rst as well? |
README is fine. You're welcome to say someone else should take it. |
@jnothman If it needs doing straight away, please feel free to pass it someone else as it will take me a while to get to it. |
Not all CIs though -- as it's still an optional dependency, and we want to check that tests can run without it. |
FYI, we added matplotlib checks for the |
We've introduced matplotlib dependency in some functions, e.g., plot_tree
(1) We need to document the min version we require (the minimal version of matplotlib for Ubuntu 16.04 is 1.5.1). Maybe also document the classes/functions which depend on matplotlib
(2) We need to install matplotlib in CIs
(3) Maybe we can remove some redundant code, e.g.,
scikit-learn/sklearn/tree/export.py
Lines 544 to 546 in 3f0c5fe
The text was updated successfully, but these errors were encountered: