Add helpful pre-commit checks and CI Documentation build step

[Jacob-Stevens-Haas]

Fixes #218 ... partially.

I used pandas' list of pre-commits to find some helpful ones, including one checking for .rst issues. Doesn't detect the binder issue or bad docstrings, unfortunately - still looking.

Also added a documentation build step to CI. I think it would be helpful - current build has a lot of broken rst, resulting in multiple of each of the following issues:

WARNING: Invalid configuration value found
ERROR: Unexpected indentation.
WARNING: Explicit markup ends without a blank line; unexpected unindent.
CRITICAL: Title level inconsistent
ERROR: Unknown interpreted text role
WARNING: image file not readable
WARNING: Inline substitution_reference start-string without end-string.
WARNING: Title underline too short.
WARNING: undefined label
WARNING: reference target not found
WARNING: Failed to create a cross reference. A title or caption not found

Leaving as draft because I can't see if the CI step works until I create a PR

[codecov-commenter]

Codecov Report

Merging #229 (79c6f58) into master (d0d96f4) will not change coverage.
The diff coverage is n/a.

❗ Current head 79c6f58 differs from pull request most recent head 63093fa. Consider uploading reports for the commit 63093fa to get more accurate results

@@           Coverage Diff           @@
##           master     #229   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files          34       34           
  Lines        3410     3410           
=======================================
  Hits         3187     3187           
  Misses        223      223           

Impacted Files	Coverage Δ
...indy/differentiation/smoothed_finite_difference.py	100.00% <ø> (ø)
pysindy/feature_library/weak_pde_library.py	95.11% <ø> (ø)
pysindy/pysindy.py	95.03% <ø> (ø)
pysindy/utils/axes.py	95.45% <ø> (ø)
pysindy/utils/base.py	83.73% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0d96f4...63093fa. Read the comment docs.

[Jacob-Stevens-Haas]

Okay sphinx build passes with no errors/warnings on my end. Doc build will automatically work for when notebooks are moved from the old format to the new one in #203. Only changes to notebooks were in fixing up header/section hierarchy: Since document header levels are determined at doc build time,

# Section
### Subsection

is equivalent to

# Section
## Subsection

The following is illegal, so I changed it in all the notebooks where it was giving a warning (again, warnings existed before - they were just ignored)

# Section
### Subsection
## Semi-sub-section???

I'd also like to ask about the Youtube.ipynb notebook in docs/. It wasn't being built in RTD, but it was detected as present and therefore sphinx gave an error about unlinked source files. So I explicitly added it to exclude in conf.py. Do we want to build docs to it and link it in examples? Or just leave it in GH but not in RTD?

Once I solve this in CI, this PR will be ready for review.

[Jacob-Stevens-Haas]

Ready for review @akaptano @znicolaou.

Summary of changes

• Added precommit hooks and fixed issues they detected:
  • whitespace errors (tabs vs spaces, EOF newline)
  • rst errors
  • common misspellings (e.g. Addtional -> Additional)
• Add doc build step to CI (raise warnings to errors)
  • Fix section title errors in README.rst and in notebooks
  • replace deprecated sphinx-nbexamples extension with nbsphinx
  • explicitly exclude Youtube.ipynb instead of just ignoring it
  • Fix notebook header errors.

Note that when converting notebooks to new format, we will need to change the links on README.rst

Instructions

To test out changes, run:

pre-commit run --all
cd docs
python -m sphinx -TEW -b html -d _build/doctrees . _build/html

Then navigate to pysindy/docs/_build/html/index.html to view webpages.

[akaptano]

Taking a look at this now. During "python -m sphinx -TEW -b html -d _build/doctrees . _build/html" I get
"""Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
File "/Users/alankaptanoglu/anaconda3/lib/python3.7/site-packages/sphinx/config.py", line 361, in eval_config_file
execfile_(filename, namespace)
File "/Users/alankaptanoglu/anaconda3/lib/python3.7/site-packages/sphinx/util/pycompat.py", line 81, in execfile_
exec(code, _globals)
File "/Users/alankaptanoglu/pysindy/docs/conf.py", line 14, in
version = release = getattr(module, "version")
AttributeError: module 'pysindy' has no attribute 'version'"""

so we could either nix this from conf.py or add a version variable to pysindy, but perhaps I'm misunderstanding

[Jacob-Stevens-Haas]

That's odd, since the relevant lines were added by Markus a long time ago (and passes on my machine). Assuming github changed double underline to bold, the __version__ attribute is added to all modules when imported based on the metadata from the install (.e.g check numpy.__version__, pandas.__version__, etc). The way the logic is supposed to go is that when you install pysindy in editable mode, the line in pyproject.toml setuptools_scm[toml]>=3.4 ensures that the metadata version always reflects the most recent git tag plus any more recent commits. So the line that errors is instead supposed to give a version like 2.0.0rc2.dev10+g97485c6', and this in turn ensures that the documentation is built with the correct version information.

Not that this would explain why your module import doesn't have a __version__ attribute, but what version of sphinx (and python) do you have installed? And how did you install pysindy in this environment?

If you're using vscode, the following debug configuration is what I use for the doc build:

        {
            "name": "Documentation",
            "type": "python",
            "request": "launch",
            "module": "sphinx.cmd.build",
            "console": "integratedTerminal",
            "justMyCode": false,
            "cwd": "${workspaceFolder}/docs",
            "args": [
                "-TE",
                "-b",
                "html",
                "-d",
                "_build/doctrees",
                ".",
                "_build/html"
            ]
        },

[akaptano]

I am using sphinx 5.1.1, Python 3.7, and the latest PySINDy main branch. Looks like I was missing some dependency in requirements-dev.txt so no longer getting that error. Now I get:sphinx.errors.SphinxWarning: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).

Warning, treated as error:
Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).

[znicolaou]

The docs build successfully in my repo, but I had to pip install nbsphinx and sudo apt install pandoc first.
PS: my sphinx is 4.4.0, and spinx-nbexamples is 0.4.1.

[Jacob-Stevens-Haas]

I am using sphinx 5.1.1, Python 3.7, and the latest PySINDy main branch. Looks like I was missing some dependency in requirements-dev.txt so no longer getting that error. Now I get:sphinx.errors.SphinxWarning: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).

Warning, treated as error: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).

Hmm, that should've been fixed when I changed this line in conf.py from

language = None

to

language = "en"

It shouldn't need setting explicitly at the command line, but if you add -D language=en to the sphinx command, do the docs build correctly?

The docs build successfully in my repo, but I had to pip install nbsphinx and sudo apt install pandoc first.

I added nbsphinx to the requirements-dev.txt, but nbsphinx requires pandoc, unfortunately. I assume readthedocs has that, but I'm not sure and I don't know how to make RTD do a test run on this branch. @akaptano is it possible to add me as a maintainer on the RTD account for pysindy? In the interim I will try building from this repo in a separate test RTD site.

[Jacob-Stevens-Haas]

Verified passes on RTD

[akaptano]

@Jacob-Stevens-Haas Any idea why the linting failed here? Looks like the sphinx-listing is off

[akaptano]

@Jacob-Stevens-Haas Wondering if you don't mind taking a look at the sphinx linting on the main branch in the next week or two -- this has been broken since this merge but is not high priority. Hope you're doing well!

[Jacob-Stevens-Haas]

Yeah definitely, I can take a look today. Sorry for going incommunicado for a couple weeks - general exam is this Thursday.

[Jacob-Stevens-Haas]

Sorry it took so long - covid got me after my general. See #254