DOC: repo-wide documentation consistency audit - #864
Conversation
|
@oyamad I am working through my review of this so I have left this in draft for now. But it is looking like a good update. |
There was a problem hiding this comment.
Pull request overview
This PR performs a repo-wide documentation consistency pass across Sphinx configuration, the apidoc generator, docstrings/doctest snippets, and repo prose docs, and also introduces a docs build job in CI to prevent future doc drift.
Changes:
- Fixes Sphinx configuration so autodoc imports the in-repo
quantecon(not an installed wheel) and improves cross-references via intersphinx. - Extends
docs/qe_apidoc.pyand committed.rstsources to include thequantecon.timingsAPI, and fixessrccleanto fully remove generated sources. - Adds a docs CI job to regenerate + drift-check committed
.rstsources and build HTML docs.
Reviewed changes
Copilot reviewed 99 out of 99 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates badges, links, install notes, citation |
| quantecon/util/timing.py | Updates timeit docstring/examples (doctest risk) |
| quantecon/util/notebooks.py | Clarifies docstring parameters/examples |
| quantecon/util/common_messages.py | Removes unused module |
| quantecon/util/combinatorics.py | Fixes doctest imports |
| quantecon/util/array.py | Fixes deprecation directive + doc typos |
| quantecon/timings/timings.py | Adds timings precision examples/output |
| quantecon/tests/util.py | Updates reference URL |
| quantecon/tests/test_inequality.py | Updates reference URL |
| quantecon/robustlq.py | Updates deprecation shim wording |
| quantecon/rank_nullspace.py | Updates deprecation shim wording |
| quantecon/random/utilities.py | Fixes doctests + doc typos/clarifications |
| quantecon/random/init.py | Documents/export draw |
| quantecon/quadsums.py | Updates deprecation shim wording |
| quantecon/quad.py | Fixes doc typos and clarifies parameters/returns |
| quantecon/optimize/tests/test_linprog_simplex.py | Updates reference URL |
| quantecon/optimize/scalar_maximization.py | Clarifies parameter defaults/order |
| quantecon/optimize/nelder_mead.py | Improves docs + doctest stability |
| quantecon/optimize/linprog_simplex.py | Fixes doc inconsistencies (phase flags etc.) |
| quantecon/optimize/lcp_lemke.py | Stabilizes doctest output type |
| quantecon/matrix_eqn.py | Updates deprecation shim wording |
| quantecon/markov/utilities.py | Fixes doctest imports |
| quantecon/markov/random.py | Fixes doctest formatting/imports |
| quantecon/markov/gth_solve.py | Fixes doc typo (“jit-compiled”) |
| quantecon/markov/estimate.py | Docstring formatting + adds missing parameter docs |
| quantecon/markov/ddp.py | Clarifies max_iter semantics + adds method doc |
| quantecon/markov/core.py | Doc clarifications + adds missing Parameters section |
| quantecon/markov/approximation.py | Fixes parameter order text + doctest stability |
| quantecon/markov/_ddp_linprog_simplex.py | Fixes doc typos/shape docs |
| quantecon/lss.py | Updates deprecation shim wording |
| quantecon/lqnash.py | Updates deprecation shim wording |
| quantecon/lqcontrol.py | Updates deprecation shim wording |
| quantecon/lae.py | Updates deprecation shim wording |
| quantecon/kalman.py | Updates deprecation shim wording |
| quantecon/ivp.py | Updates deprecation shim wording |
| quantecon/inequality.py | Updates deprecation shim wording |
| quantecon/gridtools.py | Updates deprecation shim wording |
| quantecon/graph_tools.py | Updates deprecation shim wording |
| quantecon/game_theory/vertex_enumeration.py | Fixes doctest imports/output stability |
| quantecon/game_theory/support_enumeration.py | Fixes doctest imports |
| quantecon/game_theory/repeated_game.py | Corrects return/shape docs |
| quantecon/game_theory/random.py | Fixes “Returns” section header |
| quantecon/game_theory/pure_nash.py | Fixes tol doc + doctest import |
| quantecon/game_theory/polymatrix_game.py | Replaces placeholder docstring with full docs |
| quantecon/game_theory/normal_form_game.py | Fixes doctest imports/formatting + doc clarity |
| quantecon/game_theory/mclennan_tourky.py | Fixes doctest variable name + formatting |
| quantecon/game_theory/logitdyn.py | Fixes parameter/attribute docs |
| quantecon/game_theory/lemke_howson.py | Fixes doctest imports/output + adds return docs |
| quantecon/game_theory/game_generators/bimatrix_generators.py | Normalizes doctest array formatting/imports |
| quantecon/game_theory/game_converters.py | Adds missing Parameters/Returns blocks |
| quantecon/game_theory/fictplay.py | Fixes doc typos and type docs |
| quantecon/filter.py | Updates deprecation shim wording |
| quantecon/estspec.py | Updates deprecation shim wording |
| quantecon/ecdf.py | Updates deprecation shim wording |
| quantecon/dle.py | Updates deprecation shim wording |
| quantecon/distributions.py | Removes incorrect params + fixes return doc spacing |
| quantecon/discrete_rv.py | Updates deprecation shim wording |
| quantecon/compute_fp.py | Updates deprecation shim wording |
| quantecon/ce_util.py | Updates deprecation shim wording |
| quantecon/arma.py | Updates deprecation shim wording |
| quantecon/_robustlq.py | Fixes return-order docs + type correction |
| quantecon/_rank_nullspace.py | Fixes rank/nullspace definition text |
| quantecon/_quadsums.py | Docstring cleanup + correct reference |
| quantecon/_matrix_eqn.py | Docstring cleanup + default corrections |
| quantecon/_lss.py | Fixes param name + doc typos |
| quantecon/_lqnash.py | Fixes shape docs + typo |
| quantecon/_lqcontrol.py | Fixes incorrect matrix description |
| quantecon/_lae.py | Stabilizes doctest by checking shape |
| quantecon/_kalman.py | Fixes typo + adds missing Returns docs |
| quantecon/_ivp.py | Fixes class name refs + doc formatting |
| quantecon/_inequality.py | Fixes doctest to call correct function |
| quantecon/_gridtools.py | Fixes doctest imports/output stability + typos |
| quantecon/_graph_tools.py | Fixes typo (“these”) |
| quantecon/_filter.py | Fixes typo + wraps lines |
| quantecon/_dle.py | Normalizes parameter names/types in docs |
| quantecon/_compute_fp.py | Fixes docstring quoting typo |
| quantecon/_arma.py | Fixes typo + adds missing Returns docs |
| pyproject.toml | Raises requires-python to >=3.12 |
| MANIFEST.in | Removes unused MANIFEST.in |
| docs/sphinxext/only_directives.py | Removes unused vendored extension |
| docs/sphinxext/ipython_directive.py | Removes unused vendored extension |
| docs/sphinxext/ipython_console_highlighting.py | Removes unused vendored extension |
| docs/source/util/common_messages.rst | Removes docs page for deleted module |
| docs/source/util.rst | Removes common_messages from toctree |
| docs/source/timings/timings.rst | Adds committed timings API page |
| docs/source/timings.rst | Adds timings section to docs |
| docs/source/setup.rst | Updates PyPI + Git docs links |
| docs/source/index.rst | Adds timings section + clarifies util content |
| docs/source/game_theory.rst | Title-case section heading |
| docs/source/contributing.rst | Replaces dead Discourse link with issue tracker |
| docs/source/conf.py | Fixes sys.path import root + intersphinx mappings |
| docs/rtd-requirements.txt | Unpins Sphinx version |
| docs/README.md | Updates doc build instructions |
| docs/qe_apidoc.py | Fixes generator dirs + adds timings section |
| docs/Makefile | Fixes srcclean to remove more generated sources |
| CHANGELOG.md | Fixes dates and removes dead link markup |
| .github/workflows/ci.yml | Adds docs build + docs/source drift gate |
| .github/workflows/ci_np2.yml | Fixes pull_request trigger branch |
| .github/copilot-instructions.md | Updates repo guidance to match CI/docs reality |
Comments suppressed due to low confidence (1)
quantecon/util/timing.py:350
- This
timeit(add_func, runs=2)example prints output but the docstring provides no expected output, so doctest will fail. Make the call silent (or add expected output with ellipsis).
oyamad
left a comment
There was a problem hiding this comment.
I found one issue to comment on while I was just browsing.
Addresses @oyamad's review on #864: seeding a doctest through NumPy's global random state is not a convention we want in the published docs. Replaces the `np.random.seed(1234)` in `draw` and the nine bare-int `random_state=1234` arguments with an explicit `rng = np.random.default_rng(1234)` passed as `random_state=rng`, so every reproducible example in the package now uses the same modern Generator-based pattern already used by `game_theory.random` and `markov.approximation`. `default_rng` seeds PCG64 via SeedSequence rather than the legacy Mersenne Twister, so the streams differ and every expected output was regenerated by running the example rather than by editing the old values. `random.draw` takes no `random_state` argument and draws from the global state, so its example cannot be seeded this way. It now asserts properties of the sample instead of exact values, with a Notes section recording the limitation. Extending `draw` to accept a generator, as suggested in review, is a behaviour change left for separate work. Verified: 600 tests pass; doctests pass for all five modules that use a generator in their examples; flake8 output unchanged from baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Timer and timeit docstring examples show elapsed times that cannot be reproduced exactly, so presenting them as literal expected output is misleading. Note up front that the values are illustrative and vary with machine and load. Also: - replace the fabricated "Average time: 0.0120 seconds" with a value the example actually produces - show the output of the final timeit(add_func, runs=2) example, which prints but previously displayed nothing, unlike every other example in the same docstring - add the missing "import time" so the snippets are copy-pasteable - drop trailing whitespace on the touched lines Addresses Copilot review feedback on #864. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| True | ||
| >>> qe.random.draw(cdf, 10).shape | ||
| (10,) | ||
| >>> np.random.seed(1234) |
There was a problem hiding this comment.
In my opinion, this is not a good convention. (We should not introduce doctest until #916 is implemented.)
There was a problem hiding this comment.
Fair, and agreed it shouldn't become the convention — documenting the plan here so the thread has it.
The real fix is #917 (implementing #916): draw gains support for a passed-in np.random.Generator, and this example gets rewritten there to the same rng = np.random.default_rng(1234) pattern the rest of the module moved to in this PR — at which point the np.random.seed line and the Notes block below it both disappear. The seed line is a stopgap: with the current signature it's the only way the example can show real, reproducible output rather than property checks.
On the parenthetical — agreed, and it's how the PRs are sequenced: #920 (the doctest CI job) is stacked behind this PR and queued behind #917, so this example will never be enforced as a doctest in its seeded form. Intended landing order: this PR → #917 → #920.
There was a problem hiding this comment.
I still wonder why we want to include this "antipattern" even if it is temporary. Right now draw does not offer an option to generate reproducible outputs (in my opinion np.random.seed(1234) is not a choice), so the example should be as such, I suppose?
| >>> np.random.seed(1234) | ||
| >>> qe.random.draw(cdf, 10) | ||
| array([0, 1, 1, 1, 1, 0, 0, 1, 1, 1]) | ||
| >>> int(qe.random.draw(cdf)) |
There was a problem hiding this comment.
Do we encourage the users to use int()? (Is np.int64(0) something we want to avoid?)
There was a problem hiding this comment.
I agree. I think an object repr is OK here.
There was a problem hiding this comment.
Good suggestion — adopted as the PR-wide convention in a99927a. This was one of four sites wrapping scalar outputs, so all four now show the object repr: np.int64(2) (cartesian_nearest_index), np.uint64(7) (_ints_arr_to_bits), np.float64(0.0) (lcp_lemke), and np.int64(0) here — each output verified by running the example. It's honest about what the functions actually return, and with CI and the docs build on NumPy 2 there's no portability cost. #917 will carry the same convention into the rewritten draw example.
Systematic audit of the documentation surface (Sphinx sources, the apidoc generator, package docstrings, repo prose, build config and hyperlinks), plus the fixes it turned up. Docs build - conf.py put a cwd-relative path on sys.path. Sphinx runs from docs/, so it resolved to a nonexistent directory and autodoc fell through to whatever quantecon was installed in site-packages rather than the checkout. Read the Docs has therefore been publishing docs built from the installed wheel. Now __file__-relative. - html_static_path = [''] copied the whole confdir (72 .rst files + conf.py) into the published _static/. Now []. - Added numpy/scipy to intersphinx_mapping. Generator - qe_apidoc.py never created source/game_theory/game_generators/, so the documented `make srcclean && python qe_apidoc.py` rebuild crashed with FileNotFoundError; srcclean also skipped source/optimize. - quantecon.timings is a top-level export that was documented nowhere. It now gets a generated section. Docstrings - robust_rule, robust_rule_simple and evaluate_F in _robustlq.py documented return orders that contradict their return statements. - nnash in _lqnash.py had the S1/S2 shapes swapped; the tests miss it because they use k_1 == k_2. - ~90 further parameter/return/default/type corrections, numpydoc structural fixes, and typo corrections across the package. Examples - A --doctest-modules sweep found 31 broken examples (stale pre-NumPy-1.14 array spacing, NumPy 2 scalar reprs, missing imports, and two NameErrors). All 31 fixed and verified by execution. Per module in isolation, 83 of 85 modules are now doctest-clean; the two exceptions need the network and the wall clock, and were left untouched rather than masked with doctest directives. Prose and metadata - CI matrix (was 3.11-3.13, actually 3.12-3.14), test count (was 536, actually 600), the README source-install block (ran flit with no cd, so it failed), the BibTeX entry (carried the Zenodo DOI on a JOSS @Article), and ~10 dead or redirecting URLs including two NXDOMAIN hosts. - The 21 deprecation shims said "will be removed v0.8.0"; v0.8.0 shipped in February 2025. Now version-agnostic. A removal schedule is a separate discussion. Behaviour changes worth calling out in review - requires-python: >=3.7 -> >=3.12, matching the classifiers, CI and RTD. This narrows installability. - Removed quantecon/util/common_messages.py. It was referenced nowhere and described a Numba fallback that no longer exists, but it was importable. Also adds a docs job to CI (nothing built the docs, which is how the conf.py and srcclean breakage survived), fixes the unexpanded $default-branch token in ci_np2.yml, unpins sphinx<=6.2.1 now that #707 is closed, and deletes the dead docs/sphinxext/ and MANIFEST.in. The six autodoc warnings that remain are the open question in #853 and are deliberately untouched here. Refs #853. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses @oyamad's review on #864: seeding a doctest through NumPy's global random state is not a convention we want in the published docs. Replaces the `np.random.seed(1234)` in `draw` and the nine bare-int `random_state=1234` arguments with an explicit `rng = np.random.default_rng(1234)` passed as `random_state=rng`, so every reproducible example in the package now uses the same modern Generator-based pattern already used by `game_theory.random` and `markov.approximation`. `default_rng` seeds PCG64 via SeedSequence rather than the legacy Mersenne Twister, so the streams differ and every expected output was regenerated by running the example rather than by editing the old values. `random.draw` takes no `random_state` argument and draws from the global state, so its example cannot be seeded this way. It now asserts properties of the sample instead of exact values, with a Notes section recording the limitation. Extending `draw` to accept a generator, as suggested in review, is a behaviour change left for separate work. Verified: 600 tests pass; doctests pass for all five modules that use a generator in their examples; flake8 output unchanged from baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Timer and timeit docstring examples show elapsed times that cannot be reproduced exactly, so presenting them as literal expected output is misleading. Note up front that the values are illustrative and vary with machine and load. Also: - replace the fabricated "Average time: 0.0120 seconds" with a value the example actually produces - show the output of the final timeit(add_func, runs=2) example, which prints but previously displayed nothing, unlike every other example in the same docstring - add the missing "import time" so the snippets are copy-pasteable - drop trailing whitespace on the touched lines Addresses Copilot review feedback on #864. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Notes block added in 63d784b was wrong in both halves. `draw` does not always draw from NumPy's global random state -- under jit it draws from Numba's separate internal state -- and its output can be made reproducible, from either path: pure Python : np.random.seed(1234) at Python level jitted : np.random.seed(1234) inside the jitted function Both give [0 1 1 1 1 0 0 1 1 1] for size=10, stable across processes. Numba's internal generator is Mersenne Twister, stream-compatible with NumPy's legacy global state, so the two paths agree. Because reproducibility is available, the property-checking example (`draw(cdf) in (0, 1)`, `.shape`) is no longer needed and is replaced with one that shows the values the function actually returns. The scalar draw is wrapped in `int()` because the bare NumPy 2 repr is `np.int64(0)`. `np.random.seed` here is not the convention objected to in review: `draw` accepts no `random_state`, so seeding is the only reproducibility mechanism it has. Extending `draw` to accept a generator is tracked separately.
Adopts the object-repr convention from review as the PR-wide style: all four examples that wrapped a scalar output now show the repr of what the function actually returns. Outputs verified by running each example; doctests pass for all four modules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wires the docstring-example sweep from gh-866 into ci.yml so examples cannot silently rot again after the gh-864 fixes. A new quantecon/conftest.py resolves the two blockers raised in gh-866: - An autouse fixture restores NumPy print options after each test, so the game_theory examples that set precision=4 no longer leak process-global state into doctests collected later (previously 8 spurious failures in markov, optimize and random). - collect_ignore excludes util/notebooks.py (fetches over the network) and util/timing.py (prints wall-clock durations), keeping the rendered docs free of `# doctest: +SKIP` directives. The CI step runs on Linux only: one platform is enough to stop drift, and it sidesteps --ignore-glob path-separator issues on Windows. The DeprecationWarning from util/array.py::searchsorted needs no handling since no strict warning filter is configured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a99927a to
e7b4038
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 98 out of 98 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
pyproject.toml:25
requires-pythonis raised to ">=3.12", which is a breaking distribution change for users still pinned to Python 3.7–3.11. Please ensure this is clearly called out in the next release notes (and/or whichever release process the project uses) so downstreams understand why resolution/install stopped working on older interpreters.
dynamic = ["description", "version"]
requires-python = ">=3.12"
dependencies = [
- CI docs job: also fail the drift check on untracked generated pages (git diff alone misses a brand-new .rst), drop the no-op --keep-going (only meaningful with -W), and upload the built HTML as an artifact for reviewer preview - game_theory examples: add the missing numpy/function imports so every example is copy-paste runnable, matching the convention applied elsewhere in this PR - _dle.py: document the ts_length default in irf() - test_inequality.py: cite the published article year (2019) to match the AER link, and drop a stray quote - markov/estimate.py: put Returns before Examples (numpydoc order) and make the example self-contained - docs/README.md: fix 'Do to this' typo; mark the manual qe_apidoc step as optional since 'make html' runs it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@oyamad I think this is in relatively good shape now. Thanks for your comments -- they were really helpful. Any other pickups? Otherwise I think this is a big improvement. |
Summary
A systematic audit of the documentation surface — Sphinx sources, the apidoc generator, every docstring in the package, the repo-level prose docs, build config, and every hyperlink — together with the fixes it turned up. Opened as a draft: the diff is large, and one open question (#853) is deliberately left untouched.
Findings were verified against the code rather than taken at face value, and the resulting diff was reviewed again for regressions before this PR.
Highest-impact fixes
The published docs describe the wrong package.
docs/source/conf.pyput a cwd-relative path onsys.path. Sphinx runs fromdocs/, so it resolved to a directory that does not exist, and autodoc silently fell through to whateverquanteconhappened to be installed in site-packages instead of the checkout. Read the Docs has therefore been publishing API docs generated from the installed wheel. The entry is now__file__-relative. Locally this changed the reported version in the built output from the installed0.12.0to the checkout's version.quantecon.timingswas documented nowhere. It is exported at top level and declares its own__all__, butgrep -r timings docs/returned zero hits. The generator now emits a Timings section, and the generated pages are committed.A documented rebuild path was broken.
qe_apidoc.pynever createdsource/game_theory/game_generators/, so themake srcclean && python qe_apidoc.pysequence indocs/README.mdcrashed withFileNotFoundError.srccleanalso silently skippedsource/optimize, so a "clean" rebuild left stale pages.Docstrings that contradict the code.
robust_rule,robust_rule_simpleandevaluate_Fin_robustlq.pyeach documented a return order that does not match theirreturnstatement.nnashin_lqnash.pyhad theS1/S2shapes swapped — the tests do not catch it because they usek_1 == k_2. Around 90 further parameter, return, default and type corrections follow, plus numpydoc structural fixes (a bare.. deprecated::and a missing blank line before a list were both docutils errors, so those admonitions never rendered).31 broken examples. A
--doctest-modulessweep found examples showing output no supported NumPy produces (pre-1.14 array spacing), NumPy 2 scalar reprs, missing imports, and two outrightNameErrors — includinglorenz(), which does not exist. All 31 were fixed and each verified by running it.Prose corrected against ground truth. The CI matrix (claimed 3.11–3.13, actually 3.12–3.14), the test count (claimed 536, actually 600), the README source-install block (ran
flit installwith nocd, so it could not work), the BibTeX entry (carried the Zenodo DOI on a JOSS@article), and around ten dead or redirecting URLs — two of which,lectures.quantecon.organddiscourse.quantecon.org, are NXDOMAIN.Please look closely at these two
Both are behaviour changes rather than documentation, and both are reasonable to push back on:
requires-python:>=3.7→>=3.12quantecon/util/common_messages.pygrephits only its own definition), and it describes a graceful Numba fallback that no longer exists, since numba is a hard dependency and__init__.pyhard-raises.Also included
Adds a docs job to CI. Nothing built the docs, which is precisely how the
conf.pyandsrccleanbreakage survived unnoticed. The job runsqe_apidoc.pyand thengit diff --exit-code -- docs/source, because RTD builds the committed.rstfiles and never runs the generator — regenerating without the drift check would pass green while RTD served stale pages. It builds without-Wfor now, since six warnings remain pending #853.Fixes the unexpanded
$default-branchstarter-template token inci_np2.yml, which meant that workflow'spull_requesttrigger never matched. Unpinssphinx<=6.2.1, added in #708 as a temporary workaround for #707, now closed — the pin held the docs on a 2023 Sphinx whose metadata tops out at Python 3.11 while RTD builds on 3.14. Deletes the deaddocs/sphinxext/(three vendored modules, all commented out ofconf.py) andMANIFEST.in(names a nonexistentLICENSE.txt, and flit never reads it). The 21 deprecation shims said "will be removed v0.8.0"; v0.8.0 shipped in February 2025, so the wording is now version-agnostic — an actual removal schedule is a separate discussion.Deliberately not addressed
The six autodoc warnings that remain are the open question in #853 and are untouched here. See #853 for new findings on it, including why emptying
ce_util.__all__is not a clean option.Six
game_theorydocstrings callnp.set_printoptions(precision=4)and never restore it. Harmless as rendered documentation, but under a package-wide doctest run it leaks into every module collected later. Left alone; tracked separately.Validation
pytest quantecon— 600 passed, 2 pre-existing warningsflake8 --select=F401,F405,E231 quantecon— cleansphinx-build— succeeds, 6 warnings, all of them Six objects in the deprecated shims' __all__ are missing from the top-level quantecon namespace #853# doctest: +SKIP, which would render visibly in the published docs.qe_apidoc.pyis idempotent and regenerates the committed.rstbyte-for-byte, so the new CI drift gate passes.quantecon/, apart from the shim deprecation strings and the removed module.🤖 Generated with Claude Code