Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-26506: hex() documentation: mention "%x" % int (Original patch by Manvi B)#2479

Closed
sharanry wants to merge 6 commits into
python:masterpython/cpython:masterfrom
sharanry:issue26506sharanry/cpython:issue26506Copy head branch name to clipboard
Closed

bpo-26506: hex() documentation: mention "%x" % int (Original patch by Manvi B)#2479
sharanry wants to merge 6 commits into
python:masterpython/cpython:masterfrom
sharanry:issue26506sharanry/cpython:issue26506Copy head branch name to clipboard

Conversation

@sharanry

@sharanry sharanry commented Jun 28, 2017

Copy link
Copy Markdown

Refs:
http://bugs.python.org/issue26506

Original patch by Manvi B

@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@sharanry sharanry changed the title bpo-26506: hex() documentation: mention "%x" % int bpo-26506: hex() documentation: mention "%x" % int (Patch by Manvi B) Jun 28, 2017
@sharanry sharanry changed the title bpo-26506: hex() documentation: mention "%x" % int (Patch by Manvi B) bpo-26506: hex() documentation: mention "%x" % int (Original patch by Manvi B) Jun 28, 2017
@Mariatta Mariatta added needs backport to 3.6 docs Documentation in the Doc dir labels Jun 28, 2017
Comment thread Doc/library/functions.rst Outdated
also :func:`format` for more information.

>>> num = 14
>>> format(num, '#b'), format(num, 'b')

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.

I suggest to remove the num variable and just write 14 directly, to simplify examples (and easier to copy/paste).

Comment thread Doc/library/functions.rst Outdated
'-0b1010'

If prefix "0b" is desired or not, you can use either of the following ways. See
also :func:`format` for more information.

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.

I suggest to move the "See also..." sentence after the examples.

Comment thread Doc/library/functions.rst Outdated
method that returns an integer.
If you want to convert an integer number to uppercase or lowercase hexadecimal
string either with prefix or not, you can use either of the following ways. See
also :func:`format` for more information.

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.

Ditto, move "See also ..." above.

Comment thread Doc/library/functions.rst Outdated
string either with prefix or not, you can use either of the following ways. See
also :func:`format` for more information.

>>> num=255

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.

Ditto: replace num by its value (and remove num).

@vstinner

Copy link
Copy Markdown
Member

"Original patch by Manvi B", hum, please put this string in the commit message (use "git commit --amend" for example to edit the commit message), and include the email: manvishab77@gmail.com.

@vstinner vstinner 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.

(Please make suggested changes.)

@Mariatta

Copy link
Copy Markdown
Member

Regarding the commit message, the core developer who merge this PR can manually type it in after they press the "Squash and merge" button :)

Update functions.rst based on recommendations from haypo.
Comment thread Doc/library/functions.rst Outdated

If prefix "0b" is desired or not, you can use either of the following ways.
See also :
func:`format` for more information.

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.

This line is strange... I suggest to put "See also ..." sentence after the following ">>> ..." examples.

Original patch by Manvi B ( manvishab77@gmail.com)
@sharanry

Copy link
Copy Markdown
Author

@Haypo I have made the changes you asked. Please review them.

Comment thread Doc/library/functions.rst Outdated
>>> f'{14:#b}', f'{14:b}'
('0b1110', '1110')

See also :

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.

I don't understand why :func:format is splitted in two lines. I expect an error when building the doc. Please don't split this statement into two lines.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't understand what you are saying. Could you please be more specific?

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.

You have ":" on line 102 and "func:format" on line 103. The documentation uses the Sphinx format. I don't think that your syntax is valid. Please write ":func:format" on the same line.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Never mind i understood what you are saying. Thanks!

Comment thread Doc/library/functions.rst Outdated
string either with prefix or not, you can use either of the following ways.
See also :

func:`format` for more information.

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.

Please move "See also .." after the example.

Comment thread Doc/library/functions.rst Outdated

If you want to convert an integer number to octal string either with prefix "0o"
or not, you can use either of the following ways. See also :func:`format` for
more information.

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.

Same here. I already asked you to do that :-(

@vstinner

Copy link
Copy Markdown
Member

@Mariatta: Would you mind to take a look to confirm that the doc change is fine. I don't know if we can "merge" the two "See also ..." sentence of the hex() doc. I don't know if t's worth it. Maybe just put them in the same paragraph?

@kushaldas

Copy link
Copy Markdown
Member

I think we should ask the original author if she wants to submit the PR.

@manvi77

manvi77 commented Jun 29, 2017

Copy link
Copy Markdown
Contributor

I am the original contributor. I will look into it again and can submit the PR.

@Mariatta

Copy link
Copy Markdown
Member

I think we should ask the original author if she wants to submit the PR.

I agree. I've just updated the Dev guide with guidance and protocol for converting the patches to PR:
https://devguide.python.org/pullrequest/#converting-an-existing-patch-from-the-b-p-o-to-github

I am the original contributor. I will look into it again and can submit the PR.

Please do.

Comment thread Doc/library/functions.rst

See also :func:`format` for more information.

See also :func:`int` for converting a hexadecimal string to an integer using a base of 16.

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.

Hexadecimal and base of 16 seems redundant

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What I meant is an integer using base of 16, for eg., int('0xff', 16). If it is redundant, I can remove it.

Comment thread Doc/library/functions.rst

If x is not a Python :class:`int` object, it has to define an __index__()
method that returns an integer.
If you want to convert an integer number to uppercase or lowercase hexadecimal

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.

. . . to an uppercase or lowercase . . .

@Mariatta

Mariatta commented Jul 6, 2017

Copy link
Copy Markdown
Member

Thanks for the PR @sharanry
However, I accepted PR #2525 from the original author. So I will close this.
I hope you'll continue working on another issue in the bug tracker and submit a different PR.
Thanks.

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

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.