Skip to content
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

[Meta-issue] Notebooks are outdated / non-runnable #3036

Open
21 of 73 tasks
thatlittleboy opened this issue Jun 23, 2023 · 9 comments
Open
21 of 73 tasks

[Meta-issue] Notebooks are outdated / non-runnable #3036

thatlittleboy opened this issue Jun 23, 2023 · 9 comments
Labels
documentation Relating to readthedocs, notebooks, and exposition in docstrings good first issue This is a fix that might be easier for someone to do as a first contribution help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@thatlittleboy
Copy link
Collaborator

thatlittleboy commented Jun 23, 2023

Background

We should also make sure that our documentation is kept up to date.

A scour through the open issues in this repo and also on StackOverflow shows that the outdated documentation (or lackthereof) is causing confusion among our users.

Just some examples:


Plan

The plan here is to thoroughly go through each and every notebook example that we have, to:

  • Run the notebook from top-to-bottom and ensure there are no errors
  • Fix any typos in the prose
  • Update the prose where necessary to provide better clarity
  • Update the code (where necessary and appropriate) to demonstrate up-to-date API & python features, such as:
    • Replacing the deprecated boston dataset
    • Using the Explanation API for plotting

While we're at it, I also propose to clean up the notebook metadata (remove kernel information, etc.) to minimize git conflicts, and also to run linters (ruff) / formatters (black) to keep the notebook code tidy.
I might formalize this in a separate issue later (outside the scope of this meta-issue).

An example of such a PR: #3037

To potential contributors (thank you in advance!), please stick to one notebook per PR when contributing.
Also, any help in updating the GPU-related notebooks would be very much appreciated.

TODO

@thatlittleboy thatlittleboy added help wanted Indicates that a maintainer wants help on an issue or pull request documentation Relating to readthedocs, notebooks, and exposition in docstrings labels Jun 23, 2023
@connortann
Copy link
Collaborator

This is a great endeavour! It might be helpful to add the guidelines for notebooks (e.g. linters, style etc) in the contributing guide, perhaps under a general section about "things in need of attention"

@connortann
Copy link
Collaborator

Another idea which could support this endeavour could be to make use of some more Sphinx features to make the notebooks a bit more navigable, for example the Gallery view. There's a guide here:

https://docs.readthedocs.io/en/stable/guides/jupyter.html

@franktoffel
Copy link

+1 as I was trying to read the docs and many images were missing

@connortann
Copy link
Collaborator

@thatlittleboy I noticed that we seem to have two separate sets of notebooks used for documentation:

  • The /docs/notebooks directory of .ipynb notebooks are hosted on ReadTheDocs
  • The /notebooks directory of html pages is hosted at shap.github.io, for example here. Theses are referenced in the Readme for example.

Shall we aim to remove this duplication, putting everything in one place in one format?

@znacer
Copy link
Contributor

znacer commented Sep 18, 2023

Hi,
I tried to update a few notebooks, but I am stuck with an error from pre-commit check (Import block is un-sorted or un-formatted). I tried to fix it by re arranging imports but it didn't work. I would be glad if one could help on this (#3250).

@connortann
Copy link
Collaborator

Hi, I tried to update a few notebooks, but I am stuck with an error from pre-commit check (Import block is un-sorted or un-formatted). I tried to fix it by re arranging imports but it didn't work. I would be glad if one could help on this (#3250).

Have you installed pre-commit in your working environment? Have a look in the contributing guide for more detailed steps, and let us know if anything isn't clear.

Your specific issue seems to be because some files have not been auto-formatted correctly. You should be able to fix your issue by running:

pre-commit run all-files

Then, commit and push your changes as usual.

Nb. alternatively, you can run the ruff linter directly on a notebook with:

ruff check path/to/notebook.ipynb

Note that you need the latest ruff version installed to lint .ipynb files.

@znacer
Copy link
Contributor

znacer commented Sep 25, 2023

Thanks ! Managed to use ruff check notebook.ipynb --fix to correct the problem.

@thatlittleboy
Copy link
Collaborator Author

@thatlittleboy I noticed that we seem to have two separate sets of notebooks used for documentation:

* The `/docs/notebooks` directory of `.ipynb` notebooks are hosted on ReadTheDocs

* The `/notebooks` directory of `html` pages is hosted at `shap.github.io`, for example [here](https://shap.github.io/shap/notebooks/tree_explainer/Census%20income%20classification%20with%20LightGBM.html). Theses are referenced in the Readme for example.

Shall we aim to remove this duplication, putting everything in one place in one format?

@connortann Sorry I missed this, yea seems like a good idea. Let's leave it as a to-do.

@Jason-gsy
Copy link

Hope that Image Multiclass Classification Benchmark Demo.ipynb can be fixed soon. OVO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relating to readthedocs, notebooks, and exposition in docstrings good first issue This is a fix that might be easier for someone to do as a first contribution help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

No branches or pull requests

5 participants