gh-109218: Improve documentation for the complex() constructor by serhiy-storchaka · Pull Request #119687 · python/cpython (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation32 Commits10 Checks35 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Few remarks:
0) This touch also bool, float and int constructors; changes looks fine, but at least the commit message could be more verbose and mention that.
- Should be merged before gh-109218: Deprecate weird cases in the complex() constructor #119620
- cmath has yet another instance of
z == z.real + z.imag*1jinvariant, mentioned in the issue thread. I think this should be fixed too, probably just by removing that line.
Comment on lines 382 to 392
| If the argument is a string, it should contain a decimal number, optionally |
|---|
| preceded by a sign, and optionally followed by the ``j`` or ``J`` suffix, |
| or a pair of decimal numbers, optionally preceded by a sign, separated by |
| the ``+`` or ``-`` operator, and followed by the ``j`` or ``J`` suffix. |
| A string representing a NaN (not-a-number), or infinity is acceptable |
| in place of a decimal number, like in :func:`float`. |
| The string can optionally be surrounded by whitespaces and the round |
| parenthesis ``(`` and ``)``, which are ignored. |
| The string must not contain whitespace between ``+``, ``-``, the ``j`` or |
| ``J`` suffix, and the decimal number. For example, ``complex('1+2j')`` is fine, but |
| ``complex('1 + 2j')`` raises :exc:`ValueError`. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of detail for the second paragraph. Users might find it easier to understand how to create complex numbers if the REPL examples came before this full specification
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the example of float() which has even more detailed description of the input format in the second paragraph. I merged it with a note about whitespaces, so all information related to parsing string to complex is now in one place.
I would be happy to make the description shorter, even if less detailed. Do you have any suggestions?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, though I would honestly also be inclined to move the float() examples higher up as well. I think it's probably correct to provide a full specification here; I just worry that it could overwhelmingly technical for Python beginners, who will probably learn best by looking at the examples.
In any event, I don't have a strong opinion; I'm happy to defer to you and Mark
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @AlexWaygood has a good point here: for readability, it would be nice to see just three examples (one for each of the three signatures) following the initial "Convert a single string or number to a complex number, or ..." line, before we get into the heavy detail of exactly which strings are permitted.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved examples for complex() and float() and added examples for int().
Co-authored-by: Alex Waygood Alex.Waygood@Gmail.com
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This is very much a judgement call and personal opinion, and I don't know how well it fits with the general direction of our documentation, but I do think it would be nice to have a small number of examples immediately following the initial "Convert a single string ..." doc line.
Comment on lines 382 to 392
| If the argument is a string, it should contain a decimal number, optionally |
|---|
| preceded by a sign, and optionally followed by the ``j`` or ``J`` suffix, |
| or a pair of decimal numbers, optionally preceded by a sign, separated by |
| the ``+`` or ``-`` operator, and followed by the ``j`` or ``J`` suffix. |
| A string representing a NaN (not-a-number), or infinity is acceptable |
| in place of a decimal number, like in :func:`float`. |
| The string can optionally be surrounded by whitespaces and the round |
| parenthesis ``(`` and ``)``, which are ignored. |
| The string must not contain whitespace between ``+``, ``-``, the ``j`` or |
| ``J`` suffix, and the decimal number. For example, ``complex('1+2j')`` is fine, but |
| ``complex('1 + 2j')`` raises :exc:`ValueError`. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @AlexWaygood has a good point here: for readability, it would be nice to see just three examples (one for each of the three signatures) following the initial "Convert a single string or number to a complex number, or ..." line, before we get into the heavy detail of exactly which strings are permitted.
Comment on lines +407 to +409
| If both arguments are complex numbers, return a complex number with the real |
|---|
| component ``real.real-imag.imag`` and the imaginary component |
| ``real.imag+imag.real``. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this will mention the deprecation if/when #119620 is merged? Or is the plan to remove the mention of allowing complex altogether?
I'd be tempted to not even document this (mis)feature, on the basis that people who already know about it already know about it, and people who don't know about it shouldn't start using it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan was to leave this until the end of the deprecation period.
I would be happy to remove this, but then we will receive another reports about wrong documentation or behavior. The old real + imag*1j was more correct for input with finite components (and ignoring corner cases with negative zero).
"Each argument may be any numeric type (including complex)." is already in the current documentation, so it is late to hide this without also changing the behavior.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best we can do is to put the deprecation note as close to this sentence, as possible:)
Comment on lines +386 to +401
| >>> complex('+1.23') |
|---|
| (1.23+0j) |
| >>> complex('-4.5j') |
| -4.5j |
| >>> complex('-1.23+4.5j') |
| (-1.23+4.5j) |
| >>> complex('\t( -1.23+4.5J )\n') |
| (-1.23+4.5j) |
| >>> complex('-Infinity+NaNj') |
| (-inf+nanj) |
| >>> complex(1.23) |
| (1.23+0j) |
| >>> complex(imag=-4.5) |
| -4.5j |
| >>> complex(-1.23, 4.5) |
| (-1.23+4.5j) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we reorder some of these so that the stranger ones come below the more common ones:
| >>> complex('+1.23') |
|---|
| (1.23+0j) |
| >>> complex('-4.5j') |
| -4.5j |
| >>> complex('-1.23+4.5j') |
| (-1.23+4.5j) |
| >>> complex('\t( -1.23+4.5J )\n') |
| (-1.23+4.5j) |
| >>> complex('-Infinity+NaNj') |
| (-inf+nanj) |
| >>> complex(1.23) |
| (1.23+0j) |
| >>> complex(imag=-4.5) |
| -4.5j |
| >>> complex(-1.23, 4.5) |
| (-1.23+4.5j) |
| >>> complex('+1.23') |
| (1.23+0j) |
| >>> complex('-4.5j') |
| -4.5j |
| >>> complex('-1.23+4.5j') |
| (-1.23+4.5j) |
| >>> complex(-1.23, 4.5) |
| (-1.23+4.5j) |
| >>> complex(1.23) |
| (1.23+0j) |
| >>> complex(imag=-4.5) |
| -4.5j |
| >>> complex('\t( -1.23+4.5J )\n') |
| (-1.23+4.5j) |
| >>> complex('-Infinity+NaNj') |
| (-inf+nanj) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this mixes examples for three fundamentally different roles: string parsing, numerical conversion and construction from components.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't immediately obvious to me that that was the reason behind your ordering, and I doubt it would be obvious to beginners either, which is why I suggested a different order of "most useful to least useful". But again, I don't have a strong opinion; your PR is certainly fine as-is :-)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also expect the most useful to the least useful examples even though it mixes the parsers.
| Convert a single string or number to a complex number, or create a |
|---|
| complex number from real and imaginary parts. |
| Examples: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Examples: |
|---|
| .. rubric:: Examples |
It may look a bit nicer, but I don't know whether you want a smaller heading here.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking how it is used in Doc/library/asyncio-api-index.rst, I do not think that it is appropriate here.
Comment on lines +386 to +401
| >>> complex('+1.23') |
|---|
| (1.23+0j) |
| >>> complex('-4.5j') |
| -4.5j |
| >>> complex('-1.23+4.5j') |
| (-1.23+4.5j) |
| >>> complex('\t( -1.23+4.5J )\n') |
| (-1.23+4.5j) |
| >>> complex('-Infinity+NaNj') |
| (-inf+nanj) |
| >>> complex(1.23) |
| (1.23+0j) |
| >>> complex(imag=-4.5) |
| -4.5j |
| >>> complex(-1.23, 4.5) |
| (-1.23+4.5j) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also expect the most useful to the least useful examples even though it mixes the parsers.
| The string can optionally be surrounded by whitespaces and the round |
|---|
| parenthesis ``'('`` and ``')'``, which are ignored. |
| The string must not contain whitespace between ``'+'``, ``'-'``, the |
| ``'j'`` or ``'J'`` suffix, and the decimal number. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this "j" vs "J" happens in several places. You could just mention (as in the float constructor) that parsing is case-insensitive:
complex('(InFiNITY+1j)') (inf+1j)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But "e" and "E" are mentioned separately in the float grammar. We can say also that all whitespaces except the leading and trailing are not acceptable, but I think it will be clearer to be a little more verbose here.
| Convert a single string or number to a complex number, or create a |
|---|
| complex number from real and imaginary parts. |
| Examples: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking how it is used in Doc/library/asyncio-api-index.rst, I do not think that it is appropriate here.
| The string can optionally be surrounded by whitespaces and the round |
|---|
| parenthesis ``'('`` and ``')'``, which are ignored. |
| The string must not contain whitespace between ``'+'``, ``'-'``, the |
| ``'j'`` or ``'J'`` suffix, and the decimal number. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But "e" and "E" are mentioned separately in the float grammar. We can say also that all whitespaces except the leading and trailing are not acceptable, but I think it will be clearer to be a little more verbose here.
Co-authored-by: Bénédikt Tran 10796600+picnixz@users.noreply.github.com
Co-authored-by: Bénédikt Tran 10796600+picnixz@users.noreply.github.com
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.) (cherry picked from commit ec1ba26)
Co-authored-by: Serhiy Storchaka storchaka@gmail.com
Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker ec1ba264607b2b7b98d2602f5536a1d02981efc6 3.12
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request
…ructor (pythonGH-119687)
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.) (cherry picked from commit ec1ba26)
Co-authored-by: Serhiy Storchaka storchaka@gmail.com
serhiy-storchaka added a commit that referenced this pull request
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.) (cherry picked from commit ec1ba26)
Co-authored-by: Serhiy Storchaka storchaka@gmail.com
serhiy-storchaka added a commit that referenced this pull request
…GH-119687) (ПР-119805)
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.) (cherry picked from commit ec1ba26)
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.)
estyxx pushed a commit to estyxx/cpython that referenced this pull request
- Remove the equivalence with real+imag*1j which can be incorrect in corner cases (non-finite numbers, the sign of zeroes).
- Separately document the three roles of the constructor: parsing a string, converting a number, and constructing a complex from components.
- Document positional-only parameters of complex(), float(), int() and bool() as positional-only.
- Add examples for complex() and int().
- Specify the grammar of the string for complex().
- Improve the grammar of the string for float().
- Describe more explicitly the behavior when real and/or imag arguments are complex numbers. (This will be deprecated in future.)