Skip to content

TST: Cover the multi-distribution branch in test_left_eigen_vec - #962

Open
techreign wants to merge 1 commit into
QuantEcon:mainfrom
techreign:fix/issue-961
Open

TST: Cover the multi-distribution branch in test_left_eigen_vec#962
techreign wants to merge 1 commit into
QuantEcon:mainfrom
techreign:fix/issue-961

Conversation

@techreign

@techreign techreign commented Sep 10, 2026

Copy link
Copy Markdown

What was wrong

Test_markovchain_stationary_distributions_KMRMarkovMatrix2 in
quantecon/markov/tests/test_core.py only ever used the irreducible KMR
sequential-move matrix (N = 27), which has exactly one stationary
distribution. Because of that:

  • The else branch of test_left_eigen_vec (the multiple-stationary-
    distributions case) was never executed by the suite.
  • The if len(stat_shape) == 1: arm in setup_method was similarly dead,
    since MarkovChain.stationary_distributions is documented as always
    ndim=2, so stat_shape always has length 2.

Both branches were correct code, just untested code masquerading as tested.

What changed

  • Factored the four existing test methods (test_markov_matrix,
    test_sum_one, test_nonnegative, test_left_eigen_vec) out into a
    shared, non-collected base class _Test_markovchain_stationary_distributions_Base.
  • Kept Test_markovchain_stationary_distributions_KMRMarkovMatrix2 as a
    subclass with the exact same N=27 KMR matrix, setup_method logic, and
    assertions as before (only the dead if len(stat_shape) == 1 arm was
    removed, since it can never be reached).
  • Added a new subclass, Test_markovchain_stationary_distributions_ReducibleMarkovMatrix,
    built on a reducible 4-state matrix with two absorbing states (states 0
    and 1) reached from two transient states (2 and 3). This gives
    stationary_distributions.shape == (2, 4), so n_stat_dists == 2 and the
    else branch of test_left_eigen_vec is now actually exercised and
    verified.

No production code in quantecon/markov/core.py changed; this is a
test-only fix, matching the "TST:" convention used for the issue.

How it was verified

python -m pytest quantecon/markov/tests/test_core.py -v
# 36 passed

To prove the previously-dead else branch is now hit, a temporary print
was added inside the else branch of test_left_eigen_vec and run with
pytest -k test_left_eigen_vec -s:

Test_markovchain_stationary_distributions_KMRMarkovMatrix2::test_left_eigen_vec PASSED
Test_markovchain_stationary_distributions_ReducibleMarkovMatrix::test_left_eigen_vec TEMP-COVERAGE-PROBE: else branch hit, n_stat_dists= 2
PASSED

confirming the KMR case takes the if branch (n_stat_dists == 1, no
print) and the new reducible-matrix case takes the else branch with
n_stat_dists == 2. The temporary print was removed before the final
commit (pytest-cov/coverage.py could not be used for this in the local
dev environment — both hit an unrelated ImportError: cannot load module more than once per process from numpy 2.x's C-extension guard when
combined with coverage's import hooks on this machine).

flake8 --select=F401,F405,E231 quantecon/markov/tests/test_core.py
# clean, no output

Fixes #961

Test_markovchain_stationary_distributions_KMRMarkovMatrix2 only ever
used the irreducible KMR matrix (N=27), which has exactly one
stationary distribution. As a result, test_left_eigen_vec's else
branch (the multiple-stationary-distributions case) and the
`if len(stat_shape) == 1` arm in setup_method were dead code never
executed by the suite.

Factor the shared test methods into a base class and add a new
Test_markovchain_stationary_distributions_ReducibleMarkovMatrix
subclass built on a reducible 4-state matrix with two absorbing
states, giving n_stat_dists == 2 and exercising the else branch.
stationary_distributions is documented as always ndim=2, so the dead
`if len(stat_shape) == 1` arm is removed rather than duplicated.

The existing KMR matrix test behavior and assertions are unchanged.

Fixes QuantEcon#961

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator

Hi @techreign — a quick process note while this is in the review queue.

QuantEcon's Code of AI Use (QEP-5, QuantEcon/qeps#13) doesn't restrict AI-assisted contributions, but it does ask that meaningful AI involvement be disclosed. We'd like it in two places: a trailer in the commit message, and a one-line note in the PR description.

The commit here already carries a Co-Authored-By: Claude Fable 5.1 trailer, so it's a small change — please add the canonical form alongside it:

Assisted-by: Claude Fable 5.1

(git commit --amend, then force-push the branch.) And add a line to the PR description saying which tools were used and for what — e.g. "Assisted by Claude Fable 5.1 for the test refactor".

The intent is that reviewers know what to expect, and that a human has chosen the task, checked the result, and will own the follow-up discussion.


Generated by Claude Code

@mmcky mmcky added the author-response Waiting on author response label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-response Waiting on author response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TST: test_left_eigen_vec's multi-distribution branch is never executed

3 participants