bpo-26506: hex() documentation: mention "%x" % int (Original patch by Manvi B) by sharanry · Pull Request #2479 · python/cpython

sharanry · 2017-06-28T16:54:19Z

Original patch by Manvi B

the-knights-who-say-ni · 2017-06-28T16:54:37Z

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!

vstinner · 2017-06-28T19:38:26Z

Doc/library/functions.rst

+   also :func:`format` for more information.
+
+      >>> num = 14
+      >>> format(num, '#b'), format(num, 'b')


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

vstinner · 2017-06-28T19:38:45Z

Doc/library/functions.rst

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


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

vstinner · 2017-06-28T19:39:27Z

Doc/library/functions.rst

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


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

vstinner · 2017-06-28T19:39:41Z

Doc/library/functions.rst

+   string either with prefix or not, you can use either of the following ways. See
+   also :func:`format` for more information.
+
+      >>> num=255


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

vstinner · 2017-06-28T19:41:25Z

"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

(Please make suggested changes.)

Mariatta · 2017-06-28T20:49:33Z

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.

vstinner · 2017-06-28T21:21:25Z

Doc/library/functions.rst

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


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

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

sharanry · 2017-06-28T21:31:32Z

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

vstinner · 2017-06-29T07:18:34Z

Doc/library/functions.rst

+      >>> f'{14:#b}', f'{14:b}'
+      ('0b1110', '1110')
+
+   See also :


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.

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

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.

Never mind i understood what you are saying. Thanks!

vstinner · 2017-06-29T07:22:55Z

Doc/library/functions.rst

+   string either with prefix or not, you can use either of the following ways.
+   See also :
+
+      func:`format` for more information.


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

vstinner · 2017-06-29T07:23:08Z

Doc/library/functions.rst

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


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

…com)

vstinner · 2017-06-29T08:07:46Z

@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 · 2017-06-29T13:33:25Z

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

manvi77 · 2017-06-29T19:41:58Z

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

Mariatta · 2017-06-30T17:12:55Z

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.

vadmium · 2017-07-01T02:26:08Z

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.


Hexadecimal and base of 16 seems redundant

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

vadmium · 2017-07-01T03:03:04Z

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


. . . to an uppercase or lowercase . . .

Mariatta · 2017-07-06T22:37:23Z

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.

Documentation at /Doc/library/functions.rst modified

81bc32e

the-knights-who-say-ni added the CLA not signed label Jun 28, 2017

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 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 added needs backport to 3.6 docs Documentation in the Doc dir labels Jun 28, 2017

vstinner reviewed Jun 28, 2017

View reviewed changes

vstinner requested changes Jun 28, 2017

View reviewed changes

Update functions.rst

7af69a3

Update functions.rst based on recommendations from haypo.

vstinner reviewed Jun 28, 2017

View reviewed changes

Update functions.rst. Original patch by Manvi B

90b328c

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

Update functions.rst. Original patch by Manvi B ( manvishab77@gmail.com)

6cba982

vstinner reviewed Jun 29, 2017

View reviewed changes

vstinner requested changes Jun 29, 2017

View reviewed changes

Update functions.rst. Original patch by Manvi B ( manvishab77@gmail.…

db3377d

…com)

vstinner approved these changes Jun 29, 2017

View reviewed changes

Update functions.rst. Original patch by Manvi B ( manvishab77@gmail.…

b0140aa

…com)

vadmium reviewed Jul 1, 2017

View reviewed changes

Mariatta closed this Jul 6, 2017

serhiy-storchaka removed the needs backport to 3.6 label Sep 27, 2017


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

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

Uh oh!

Conversation

sharanry commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Jun 28, 2017

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 commented Jun 28, 2017

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Mariatta commented Jun 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharanry commented Jun 28, 2017

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!

vstinner commented Jun 29, 2017

Uh oh!

kushaldas commented Jun 29, 2017

Uh oh!

manvi77 commented Jun 29, 2017

Uh oh!

Mariatta commented Jun 30, 2017

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!

Mariatta commented Jul 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sharanry commented Jun 28, 2017 •

edited

Loading