Skip to content

FIX: Align random.draw size dispatch for numpy integers - #919

Closed
kocaemre wants to merge 1 commit into
QuantEcon:mainfrom
kocaemre:fix/draw-numpy-integer-size
Closed

kocaemre wants to merge 1 commit into
QuantEcon:mainfrom
kocaemre:fix/draw-numpy-integer-size

Conversation

@kocaemre

@kocaemre kocaemre commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Closes #918.

Summary

  • Treat numpy integer sizes (for example np.int64(10)) the same as Python int in the pure-Python random.draw path.
  • Keep boolean size values on the scalar branch so Python and jitted dispatch agree.
  • Add focused regression tests covering Python and @njit callers for both cases.

Test plan

  • RED: pytest quantecon/random/tests/test_utilities.py::TestDraw::test_numpy_integer_size_returns_array quantecon/random/tests/test_utilities.py::TestDraw::test_bool_size_is_treated_as_scalar -q failed with the Python path returning a scalar for np.int64(10) and raising TypeError for True.
  • GREEN: same focused command passed: 2 passed in 6.43s.
  • pytest quantecon/random/tests/test_utilities.py -q11 passed in 4.45s.
  • pytest quantecon/random -q11 passed in 4.54s.
  • pytest quantecon -q602 passed, 2 warnings in 504.77s.
  • flake8 --select=F401,F405,E231 quantecon.
  • git diff --check.

Signed-off-by: Emre K <110906681+kocaemre@users.noreply.github.com>
@kocaemre
kocaemre force-pushed the fix/draw-numpy-integer-size branch from 6a3ed13 to 3c9f13f Compare August 18, 2026 10:11
@kocaemre

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current main and resolved the conflict in quantecon/random/utilities.py against the newer draw(..., rng=...) implementation.

The preserved behavior is:

  • NumPy integer sizes such as np.int64(10) take the sized array branch.
  • Boolean sizes still take the scalar branch.
  • The newer rng path is preserved, so sized draws use rng.random(size) when a generator is provided.

Local verification after the rebase:

pytest -q quantecon/random/tests/test_utilities.py
22 passed in 6.10s

flake8 --select=F401,F405,E231 quantecon/random/utilities.py quantecon/random/tests/test_utilities.py
passed

git diff --check
passed

New head: 3c9f13f.

@mmcky

mmcky commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @kocaemre — closing this one, but the fix is landing: I've opened #955 with the reduced form of it.

Two things came out of reviewing this that are worth passing back. The @overload edits turn out to be no-ops: types.Boolean is not a subclass of types.Integer, so isinstance(size, types.Integer) and not isinstance(size, types.Boolean) can never differ from isinstance(size, types.Integer) alone. The jitted path was already dispatching every case correctly — only the pure-Python body was wrong. Similarly, the np.bool_ half of the exclusion in the Python predicate cannot be reached, because np.bool_ fails isinstance(size, (int, np.integer)) first. So the change reduces to the single line #918 proposed, and #955 is that line plus your np.int64 test, which was the genuinely useful part of this PR.

A suggestion for future contributions here: when an issue already specifies the fix — #918 gave the exact predicate, the dispatch table and the reasoning — the most valuable PR is the smallest one that implements it, plus a test. Extra defensive clauses cost review time to prove inert, which is time we would rather spend on the next issue.

Thanks for the work on this, including the rebase onto the rng= path after #917 landed.

@mmcky mmcky closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

random.draw: size dispatch diverges between the Python and jitted paths for numpy integers

2 participants