Skip to content

bpo-30940: fix docs for round if second arg is None - #2824

Closed
daxlab wants to merge 4 commits into
python:masterfrom
daxlab:bpo_30940
Closed

bpo-30940: fix docs for round if second arg is None#2824
daxlab wants to merge 4 commits into
python:masterfrom
daxlab:bpo_30940

Conversation

@daxlab

@daxlabdaxlab commented Jul 23, 2017

Copy link
Copy Markdown
Contributor

@mention-bot

Copy link
Copy Markdown

@daxlab, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @benjaminp and @rhettinger to be potential reviewers.

Comment threadDoc/library/functions.rst Outdated
``2``). Any integer value is valid for *ndigits* (positive, zero, or
negative). The return value is an integer if called with one argument,
otherwise of the same type as *number*.
negative). The return value is an integer if *ndigits* is omitted or *None*.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Format None with fixed-width font instead of italic.

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needed to update the docstring too.

The signature of the function can be written as

round(number, ndigits=None)

@mdickinsonmdickinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks for doing this!

@serhiy-storchaka

serhiy-storchaka commented Jul 23, 2017

Copy link
Copy Markdown
Member

Please update the docstring.

And what is more correct,

if *ndigits* is omitted or is ``None``

(as in the first paragraph), or

if *ndigits* is omitted or ``None``

(as in the second paragraph)? Or both are correct?

@daxlab

Copy link
Copy Markdown
ContributorAuthor

if *ndigits* is omitted or ``None`` looks correct.

Comment threadPython/bltinmodule.c Outdated
"round(number, ndigits=None) -> number\n\
\n\
Round a number to a given precision in decimal digits (default 0 digits).\n\
This returns an int when called with one argument, otherwise the\n\

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"if ndigits is omitted or None"

@mdickinson

Copy link
Copy Markdown
Member

About

if ndigits is omitted or is None

versus

if ndigits is omitted or None

I think either is fine. To my eyes, the first looks a bit clearer, but also a bit more pedantic.

@daxlab

Copy link
Copy Markdown
ContributorAuthor

@serhiy-storchaka docs updated.

@daxlab

daxlab commented Aug 27, 2017

Copy link
Copy Markdown
ContributorAuthor

@serhiy-storchaka plz review. Also, should I send a separate PR to 3.6 branch or you can cherry-pick ?

@serhiy-storchaka

Copy link
Copy Markdown
Member

The current changes LGTM, but see a discussion on the tracker. Perhaps the documentation needs additional clarification.

@ned-deily

Copy link
Copy Markdown
Member

Based on more recent discussion on the tracker, it looks these proposed changes have now been addressed by other PRs, primarily #6342 and #2740. Thanks for starting the ball rolling @daxlab!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting mergedocsDocumentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@daxlab@mention-bot@serhiy-storchaka@mdickinson@ned-deily@brettcannon@the-knights-who-say-ni@bedevere-bot