GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-161452: Fix `python -m random` with a non-positive integer input by andresdelfino · Pull Request #161453 · python/cpython

Conversation

andresdelfino commented Nov 5, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

When python -m random gets a single integer input, main() called randint() inside the try block that converts the input with int(). For an integer N <= 0, the ValueError that randint() raises for an empty range was taken to mean that the input is not an integer, so python -m random 0 printed 0.0 and a negative N printed a random negative float. This keeps only the conversions inside the try blocks (the randint() and uniform() calls move to else: blocks), so an integer input now behaves like --integer N, as documented.

Run from the command line, both forms now print a one-line error and exit with status 1, instead of a traceback:

$ ./python -m random -3
error: empty range in randint(1, -3)
$ ./python -m random --integer 0
error: empty range in randint(1, 0)

main() itself still raises the ValueError; only the __main__ block catches it.

The new tests check that 0 and -5 raise the same ValueError from main() with and without --integer, and that the command line reports the short error message.

🤖 Generated with Claude Code

Issue: python -m random N prints a float for an integer N <= 0 #161452

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          pythongh-161452: Fix python -m random with a non-positive integer input

5ee6049

When given a single integer input, main() called randint() inside the
same try block that converts the input with int(). The ValueError that
randint() raises for an empty range was then taken to mean that the
input is not an integer, so "python -m random 0" printed 0.0 and a
negative N printed a random negative float, instead of failing like
"--integer N" does. Only convert the input inside the try block.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

andresdelfino requested a review from rhettinger as a code owner

November 5, 2026 22:27

bedevere-app Bot added the awaiting review label

Nov 5, 2026

bedevere-app Bot mentioned this pull request

Nov 5, 2026

python -m random N prints a float for an integer N <= 0 #161452

Open

skirpichev previously approved these changes

Nov 5, 2026

View reviewed changes

bedevere-app Bot added awaiting core review and removed awaiting review labels

Nov 5, 2026

gvanrossum reviewed

Nov 6, 2026

View reviewed changes

Comment thread

Lib/random.py Outdated

@@ -1076,6 +1075,8 @@ def main(arg_list: list[str] | None = None) -> int | str:

gvanrossum Nov 6, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you want to move this to an (inner) else: block too?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

skirpichev Nov 6, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I think it's the right place: uniform() not raises (c.f. randint).

So, here we have only parsing failures for floats. In that case it's reasonable to try choice() as a fallback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum reacted with thumbs up emoji

All reactions

👍 1 reaction

andresdelfino Nov 6, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While Sergey is right, it would be easier to read if the same pattern is followed. Let's to think of. Let me know what you prefer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum reacted with thumbs up emoji

All reactions

👍 1 reaction

vstinner Nov 6, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @gvanrossum, it's better to move return uniform(0, val) to an else: block as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andresdelfino reacted with thumbs up emoji

All reactions

👍 1 reaction

andresdelfino Nov 7, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner reviewed

Nov 6, 2026

View reviewed changes

vstinner left a comment •

edited by hugovk

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is backward incompatible, but it respects the documentation: https://docs.python.org/dev/library/random.html#command-line-usage

If no options are given, the output depends on the input:
Integer: same as --integer.

Hum. Displaying a long traceback is not really user friendly. Can you catch the exception and display a (short) error message instead?

$ ./python -m random -3
Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/runpy.py", line 201, in _run_module_as_main
    return _run_code(code, main_globals, None,
                     "__main__", mod_spec)
  File "/home/vstinner/python/main/Lib/runpy.py", line 87, in _run_code
    exec(code, run_globals)
    ~~~~^^^^^^^^^^^^^^^^^^^
  File "/home/vstinner/python/main/Lib/random.py", line 1088, in <module>
    print(main())
          ~~~~^^
  File "/home/vstinner/python/main/Lib/random.py", line 1079, in main
    return randint(1, val)
  File "/home/vstinner/python/main/Lib/random.py", line 341, in randint
    raise ValueError(f"empty range in randint({a}, {b})")
ValueError: empty range in randint(1, -3)

cc @hugovk who authored this feature.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andresdelfino reacted with thumbs up emoji

All reactions

👍 1 reaction

hugovk commented Nov 6, 2026

Copy link

Copy Markdown

Member

Yes, a short error message would be better than the traceback.

andresdelfino reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rhettinger removed their request for review

November 6, 2026 17:06


          Address review: use else block and report a short CLI error

7443cc8

Move the uniform() call to an else block too, matching the integer
branch. When run from the command line, catch the ValueError for an
empty integer range and exit with a one-line error message instead of
a traceback.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

andresdelfino commented Nov 7, 2026

Copy link

Copy Markdown

Contributor Author

@vstinner I applied your suggestion, handling the exception in the block that calls main. Changing the main function itself would involve more changes in the tests. Would it be better to go that way?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reviewed

Nov 7, 2026

View reviewed changes

Comment thread

Lib/test/test_random.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_random.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Nov 7, 2026

Copy link

Copy Markdown

Member

Some general notes on random CLI. I'm not sure that it's worth it to change them. I just wanted to share my findings.

./python -m random single_choice always returns single_choice. I suppose that it's a deliberate behavior. It's similar to ./python -m random 1 which returns always 1 and ./python -m random --float 0 which returns always 0.0.

There is a surprising behavior when the integer is larger than sys.get_int_max_str_digits(). int() raises ValueError: Exceeds the limit (4300 digits) for integer string conversion: value has 40096 digits and so switchs to float() which returns inf:

$ ./python -m random $(python -c "print('1'*40096)")
inf

$ ./python -m random $(python -c "print('-' + '1'*40096)")
-inf

$ ./python -m random --integer $(python -c "print('1'*40096)")
usage: python -m random [-h] [-c CHOICE [CHOICE ...] | -i N | -f N] [input ...]
python -m random: error: argument -i/--integer: invalid int value: '111111111111(...)111'

$ ./python -m random --integer $(python -c "print('-' + '1'*40096)")
usage: python -m random [-h] [-c CHOICE [CHOICE ...] | -i N | -f N] [input ...]
python -m random: error: argument -i/--integer: invalid int value: '-1111111111111(...)111'

By the way, it's possible to pass special float values:

$ ./python -m random --float inf
inf

$ ./python -m random --float nan
nan

It seems like random.uniform() always return inf when the maximum is inf:

$ ./python
>>> random.uniform(0, float('inf'))
inf

Ok, maybe it can return nan if self.random() returns 0.0 which is unlikely but possible :-)

And random.uniform(0, float('nan')) always return nan.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

skirpichev dismissed their stale review

November 7, 2026 02:06

stale

bedevere-app Bot added awaiting review and removed awaiting core review labels

Nov 7, 2026

andresdelfino and others added 2 commits

November 6, 2026 23:11


          Update Lib/test/test_random.py

a4053f7

Co-authored-by: Victor Stinner <[email protected]>


          Remove test_main_empty_integer_range

a61d062

vstinner approved these changes

Nov 7, 2026

View reviewed changes

vstinner left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I just have a last suggestion about adding a "real" random.main() function.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/random.py Outdated

Comment on lines +1089 to +1093

		import sys
		try:
		print(main())
		except ValueError as exc:
		sys.exit(f"error: {exc}")

vstinner Nov 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might rename main() to _main() and wrap these lines into a new main() function. So for example, you can call the new main() function in tests instead of having to spawn a new process.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

andresdelfino Nov 7, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I do like gzip instead, and call sys.exit directly from main? So we get the benefit you mention, and follow an existing pattern (personally I always find _main/main confusing).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner Nov 7, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I suggest is to have a main() function which does everything: parse command line arguments, compute a random value, display the random value, call sys.exit() on error:

if __name__ == '__main__':
    main()

I don't care too much of how main() code would look like :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

andresdelfino Nov 8, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! We were talking about the same thing :) I'll implement this, as this seems to be the common way of implementing CLIs for modules.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reacted with thumbs up emoji

All reactions

👍 1 reaction

andresdelfino Nov 8, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up following your previous _main suggestion, as a lots of returns + a few try blocks would be necessary for main to handle everything properly by itself.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting merge and removed awaiting review labels

Nov 7, 2026


          Move CLI error handling into random.main()

cc6938f

Split the CLI logic into _main(), which returns the result, and make
main() print it and exit with a short error message on failure.  Test
the error path in-process instead of spawning a subprocess.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

vstinner reviewed

Nov 8, 2026

View reviewed changes

Comment thread

Lib/test/test_random.py Outdated

		self.assertEqual(str(cm.exception),
		f"empty range in randint(1, {value})")

		def test_main_error_no_traceback(self):

vstinner Nov 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If test_empty_integer_range_error() is modified to test directly main(), I think that this test can be simply removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Misc/NEWS.d/next/Library/2026-11-05-22-26-44.gh-issue-161452.YfFfjq.rst Outdated

Comment on lines +4 to +5

		instead of a traceback. :func:`!random.main` now prints the result instead
		of returning it, and exits with the error message on failure.

vstinner Nov 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't document the random.main() function, I don't think that we should document when its behavior changes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_random.py Outdated

		]:
		random.seed(0)
		self.assertEqual(random.main(shlex.split(command)), expected)
		self.assertEqual(random._main(shlex.split(command)), expected)

vstinner Nov 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I would prefer to test the public main() function:

Suggested change

	self.assertEqual(random._main(shlex.split(command)), expected)
	with support.captured_stdout() as stdout:
	random.main(shlex.split(command))
	self.assertEqual(stdout.getvalue().rstrip(), str(expected))

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_random.py Outdated

Comment on lines +1488 to +1494

		for value in ["0", "-5"]:
		for args in [[value], ["--integer", value]]:
		with self.subTest(args=args):
		with self.assertRaises(ValueError) as cm:
		random._main(args)
		self.assertEqual(str(cm.exception),
		f"empty range in randint(1, {value})")

vstinner Nov 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer testing the public main() function, rather than testing _main() which is more an implementation detail:

Suggested change

	for value in ["0", "-5"]:
	for args in [[value], ["--integer", value]]:
	with self.subTest(args=args):
	with self.assertRaises(ValueError) as cm:
	random._main(args)
	self.assertEqual(str(cm.exception),
	f"empty range in randint(1, {value})")
	for value in ["0", "-5"]:
	err_msg = f"error: empty range in randint(1, {value})"
	for args in [[value], ["--integer", value]]:
	with self.subTest(args=args):
	with self.assertRaises(SystemExit) as cm:
	random.main(args)
	self.assertEqual(str(cm.exception), err_msg)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

hugovk commented Nov 8, 2026

Copy link

Copy Markdown

Member

I'm also not keen on _main + main, we don't have any other CLIs like this.

The traceback is now replaced with a short error:

❯ ./python.exe -m random -3
error: empty range in randint(1, -3)

But that doesn't mean much to me. What's a randint? Before I had a traceback pointing me to where it all came from so I could look it up.

So I suggest, let's keep:

if __name__ == '__main__':
    print(main())

Add this validation and call _randint_cli(parser, x) in place of plain randint(1, x) in the CLI:

def _randint_cli(parser, n: int) -> int:
    if n < 1:
        parser.error(f"integer must be 1 or greater, got {n}")
    return randint(1, n)

This mean we get the native argparse error, which prints usage, and also we get colour (and colour handling):

❯ ./python.exe -m random -3
usage: python.exe -m random [-h] [-c CHOICE [CHOICE ...] | -i N | -f N] [input ...]
python.exe -m random: error: integer must be 1 or greater, got -3

And _parse_args also needs to return args, parser instead of args, parser.format_help(), and the final return help_text becomes return parser.format_help().

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Report non-positive integers with parser.error()

bd1b41e

Keep main() returning its result as before, and have the CLI reject a
non-positive integer through argparse's parser.error(), so it prints
the usage and a short error message and exits with status 2.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

andresdelfino commented Nov 8, 2026

Copy link

Copy Markdown

Contributor Author

@hugovk I followed your suggestions

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hugovk approved these changes

Nov 8, 2026

View reviewed changes

hugovk left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_random.py Outdated

		args, parser = random._parse_args(shlex.split("--choice a b c"))
		self.assertEqual(args.choice, ["a", "b", "c"])
		self.assertStartsWith(help_text, "usage: ")
		self.assertStartsWith(parser.format_help(), "usage: ")

hugovk Nov 8, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could now remove these lines, we don't need to test argparse.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

andresdelfino Nov 8, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just pushed the change. Let me know if I understood you correctly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Do not test argparse

baa1441

Reviewers

vstinner

vstinner approved these changes

skirpichev

skirpichev left review comments

gvanrossum

gvanrossum left review comments

hugovk

hugovk approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

andresdelfino commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment • edited by hugovk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugovk commented Nov 6, 2026

Uh oh!

andresdelfino commented Nov 7, 2026

Uh oh!

Uh oh!

Uh oh!

vstinner commented Nov 7, 2026

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugovk commented Nov 8, 2026

Uh oh!

andresdelfino commented Nov 8, 2026

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andresdelfino commented Nov 5, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

vstinner left a comment •

edited by hugovk

Loading

Uh oh!

There was an error while loading. Please reload this page.