DEPR: deprecate Matlab-like tic/tac/toc/loop_timer timers (#786) - #833
Open
Builder106 wants to merge 3 commits into
Open
DEPR: deprecate Matlab-like tic/tac/toc/loop_timer timers (#786)#833Builder106 wants to merge 3 commits into
Builder106 wants to merge 3 commits into
Conversation
Contributor
|
Hi @Builder106 — this is really cleanly done, thanks! 🙏 The On your open question about the target version: let's plan to remove these in v1.0 rather than 0.12.0. There's no harm in carrying them with the deprecation notice in the meantime, and v1.0 is the natural point to drop the Matlab-style API. One small distinction while you make that change: the
That way the rendered docs read "deprecated since 0.12.0, will be removed in 1.0", which is the conventional phrasing. Everything else looks great — happy to approve once that's tweaked. Thanks again! 😊 |
) Emit a DeprecationWarning from the public `tic`, `tac`, `toc` and `loop_timer` functions, pointing users to the modern `Timer` context manager and `timeit` function added in QuantEcon#783/QuantEcon#794. Removal is targeted for v0.12.0. - `__Timer__.loop_timer` now calls its own methods (`self.tic`/`tac`/ `toc`) instead of the public wrappers, so a single `loop_timer` call emits exactly one warning rather than one per run. - The warning uses `stacklevel=3` so it is attributed to the user's call site rather than the wrapper. - Added `.. deprecated:: 0.12.0` directives to the module and the affected docstrings. - Added tests asserting each function warns, that `loop_timer` warns only once, and that the warning points at the caller. Note: the original issue mentioned removal in 0.9.*, but the library is already at v0.11.2, so this targets v0.12.0 instead.
Builder106
force-pushed
the
deprecate-tic-tac-toc
branch
from
July 18, 2026 22:41
a1bcf96 to
387ee1e
Compare
This was referenced Sep 9, 2026
|
coverage: 90.895% (+0.01%) from 90.884% — SankofaForge:deprecate-tic-tac-toc into QuantEcon:main |
Collaborator
|
Hi @Builder106 👋 Thanks again for this PR — it's in good shape and I've opened a small stacked follow-up in #958 with a few review nits (docstring wording and flattening the test warning filters), so nothing further is needed from you on those. One friendly request. QuantEcon is currently drafting an AI disclosure policy for contributions (see QuantEcon/qeps#13). It isn't in force yet, but it would help us as we shape the policy if you could add a short note to the PR description saying whether AI tools were used in preparing this PR and, if so, which tools or models (e.g. "drafted with Claude Code / Copilot / ChatGPT, then reviewed and tested by hand"). "No AI tools used" is an equally welcome answer. The goal is transparency rather than gatekeeping — AI-assisted contributions are fine, we'd just like the provenance recorded. Generated by Claude Code |
mmcky
added a commit
that referenced
this pull request
Sep 9, 2026
The index page's description of util listed tic, tac and toc alongside Timer and timeit. Those three are being deprecated in #833, so the landing page should not advertise them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Sep 9, 2026
* DOC: repo-wide documentation consistency audit 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> * DOC: standardise docstring examples on np.random.default_rng 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> * DOC: timing: mark timing example output as illustrative 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> * DOC: correct draw() RNG notes and restore a concrete example 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. * DOC: show NumPy scalar reprs instead of int()/float() wrappers 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> * DOC/CI: final-review touch-ups - 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> * DOC: Show the simulated path in the discrete_var example The example had been reduced to checking the shape of mc.simulate's output. The output is deterministic under the seeded generator, and the same example already prints exact state_values drawn from the same stream, so show the values the reader would actually see. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * CI: Use actions/setup-python@v7 in the docs job Matches the version the rest of ci.yml already uses after Dependabot's bump on main. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * DOC: Name only Timer and timeit as the user-facing timing tools The index page's description of util listed tic, tac and toc alongside Timer and timeit. Those three are being deprecated in #833, so the landing page should not advertise them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #786 (Step 1).
Now that the
Timercontext manager (#783) andtimeit(#794) are merged, this implements Step 1 of #786: emit aDeprecationWarningfrom the legacy Matlab-like timers, steering users to the modern API. Step 2 (removing the functions) can follow in the targeted release.Changes
tic,tac,toc, andloop_timernow emit aDeprecationWarning.tic/tac/tocpoint to theTimercontext manager;loop_timerpoints totimeit.stacklevel=3, so it's attributed to the user's call site rather than the internal wrapper.__Timer__.loop_timernow calls its own methods (self.tic()/self.tac()/self.toc()) instead of the public wrapper functions. This is functionally identical but ensures a singleloop_timercall emits exactly one warning instead of one per run... deprecated:: 0.12.0directives to the module docstring and the affected function/method docstrings.Tests
TestDeprecationclass asserting each function raisesDeprecationWarning, thatloop_timerwarns only once (no nested warnings), and that the warning points at the caller's file.33 passed).Note on target version
The issue text mentions removal for
0.9.*, but the library is already at v0.11.2, so the deprecation message and directives target v0.12.0. Happy to adjust to whatever removal version the maintainers prefer.