Skip to content

/ scikit-learn

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

Jump to bottom

MNT Use substitutions for minimum dependencies in documentation #17931

Merged
merged 3 commits into from Jul 15, 2020
Merged

MNT Use substitutions for minimum dependencies in documentation #17931

merged 3 commits into from Jul 15, 2020

Conversation

@alfaro96
Copy link
Member

@alfaro96 alfaro96 commented Jul 15, 2020

Reference Issues/PRs

Similar to #17573.

What does this implement/fix? Explain your changes.

This PR uses rst substitutions to specify the minimum dependencies inside the documentation.

Any other comments?

CC @thomasjpfan.
alfaro96 added 3 commits Jul 15, 2020
@alfaro96
MNT Use substitutions for minimum dependencies in documentation
Unverified
This user has not uploaded their public key yet.
GPG key ID: 4D76EF9472F3A6D1 Learn about signing commits
Loading status checks…
6af7393
@alfaro96
Trigger build again
Unverified
This user has not uploaded their public key yet.
GPG key ID: 4D76EF9472F3A6D1 Learn about signing commits
Loading status checks…
3ba3aaa
@alfaro96
FIX Add forgotten dots to define substitution
Unverified
This user has not uploaded their public key yet.
GPG key ID: 4D76EF9472F3A6D1 Learn about signing commits
Loading status checks…
5de58dd
@thomasjpfan
thomasjpfan approved these changes Jul 15, 2020
View changes
Copy link
Member

@thomasjpfan thomasjpfan left a comment

This looks good.

One of the last few places we have hard coded versions is README.rst. (I do not know we can do anything about that)
@rth
rth approved these changes Jul 15, 2020
View changes
Copy link
Member

@rth rth left a comment

Great, thanks!
@rth rth merged commit 682bf3c into scikit-learn:master Jul 15, 2020
21 checks passed
21 checks passed
LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No new or fixed alerts
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
ci/circleci: doc artifact Link to 0/doc/_changed.html
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing e97fd14...5de58dd
Details
codecov/project 98.07% (+0.00%) compared to e97fd14
Details
scikit-learn.scikit-learn Build #20200715.21 succeeded
Details
scikit-learn.scikit-learn (Linting) Linting succeeded
Details
scikit-learn.scikit-learn (Linux py36_conda_openblas) Linux py36_conda_openblas succeeded
Details
scikit-learn.scikit-learn (Linux py36_ubuntu_atlas) Linux py36_ubuntu_atlas succeeded
Details
scikit-learn.scikit-learn (Linux pylatest_pip_openblas_pandas) Linux pylatest_pip_openblas_pandas succeeded
Details
scikit-learn.scikit-learn (Linux32 py36_ubuntu_atlas_32bit) Linux32 py36_ubuntu_atlas_32bit succeeded
Details
scikit-learn.scikit-learn (Linux_Runs pylatest_conda_mkl) Linux_Runs pylatest_conda_mkl succeeded
Details
scikit-learn.scikit-learn (Windows py36_pip_openblas_32bit) Windows py36_pip_openblas_32bit succeeded
Details
scikit-learn.scikit-learn (Windows py37_conda_mkl) Windows py37_conda_mkl succeeded
Details
scikit-learn.scikit-learn (macOS pylatest_conda_mkl) macOS pylatest_conda_mkl succeeded
Details
scikit-learn.scikit-learn (macOS pylatest_conda_mkl_no_openmp) macOS pylatest_conda_mkl_no_openmp succeeded
Details
@alfaro96
Copy link
Member Author

@alfaro96 alfaro96 commented Jul 16, 2020

This looks good.

One of the last few places we have hard coded versions is README.rst. (I do not know we can do anything about that)

I have been deeply thinking about this issue. However, I have not found a solution to avoid hardcoded versions in README.rst. The original idea was to have an external repository (scikit-learn-min-dependencies) with a file containing the minimum dependencies (scikit_learn_min_dependencies.rst) and updated according to a cron job. Then, to include this file (.. include) within the README.rst and use substitutions. Nevertheless, including a file from an external repository is not allowed.

Thus, I have realized that may be a good solution would be to have a GitHub workflow that automatically creates a PR updating the dependencies of the README.rst file based on modifications to the /sklearn/_build_utils/min_dependencies.py file.

WDYT?
@alfaro96 alfaro96 deleted the alfaro96:min_dependencies_doc branch Jul 16, 2020
@jnothman
Copy link
Member

@jnothman jnothman commented Jul 16, 2020

@alfaro96 alfaro96 mentioned this pull request Aug 30, 2020
Merged
jayzed82 added a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@alfaro96 @jayzed82
MNT Use substitutions for minimum dependencies in documentation (scik…
e7f2696 
…it-learn#17931)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rth rth

@thomasjpfan thomasjpfan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@alfaro96 @jnothman @rth @thomasjpfan