| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
I'm neutral on this change but I think it's fine to keep it as it can be used without any import.
Sorry, something went wrong.
|
I don't think it's a good idea. Example in the current main is clear and short. The stdlib here is to be used, if people don't know about operator.mul - it's a good opportunity to learn. |
Sorry, something went wrong.
|
@AlexWaygood Since you reacted with 👍, what do you think? @python/editorial-board What would be the recommendation in general for such snippets? do we want to introduce more notion in a "roughly equivalent" snippet or should such snippets be as pure as possible (and copy-pastable directly?) |
Sorry, something went wrong.
|
Honestly I don’t think this is a good question for the EB, each case is
likely different.
…--Guido (mobile)
On Sun, Feb 16, 2025 at 16:25 Bénédikt Tran ***@***.***> wrote:
@AlexWaygood <https://github.com/AlexWaygood> Since you reacted with 👍,
what do you think?
@python/editorial-board
<https://github.com/orgs/python/teams/editorial-board> What would be the
recommendation in general for such snippets? do we want to introduce more
notion in a "roughly equivalent" snippet or should such snippets be as pure
as possible (and copy-pastable directly?)
—
Reply to this email directly, view it on GitHub
<#130206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWCWMT7PQH7IFOI4BWA2BT2QEUAXAVCNFSM6AAAAABXH3RKFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRRG4YDKNBWGI>
.
You are receiving this because you are on a team that was mentioned.Message
ID: ***@***.***>
[image: picnixz]*picnixz* left a comment (python/cpython#130206)
<#130206 (comment)>
@AlexWaygood <https://github.com/AlexWaygood> Since you reacted with 👍,
what do you think?
@python/editorial-board
<https://github.com/orgs/python/teams/editorial-board> What would be the
recommendation in general for such snippets? do we want to introduce more
notion in a "roughly equivalent" snippet or should such snippets be as pure
as possible (and copy-pastable directly?)
—
Reply to this email directly, view it on GitHub
<#130206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWCWMT7PQH7IFOI4BWA2BT2QEUAXAVCNFSM6AAAAABXH3RKFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRRG4YDKNBWGI>
.
You are receiving this because you are on a team that was mentioned.Message
ID: ***@***.***>
|
Sorry, something went wrong.
Fair enough. Sorry for the ping! For me, I'm fine to merge this change so that one can just C/C the code without the needs to import operator, but I'm also fine with keeping the operator since it gives the possibility of knowing about operator.mul and map(strict=True) (this one is for me). So, while I approved the PR, I'll wait for Alex's opinion as well. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for the PR @Akuli and the reviews @picnixz @AlexWaygood @skirpichev.
Sorry, something went wrong.
| Roughly equivalent to:: | ||
|
|
||
| sum(map(operator.mul, p, q, strict=True)) | ||
| sum(px * qx for px, qx in zip(p, q, strict=True)) |
There was a problem hiding this comment.
I would suggest leaving as is but adding a sentence below such as:
sum(map(operator.mul, p, q, strict=True)) where the multiply operator is used to multiply each item in the *p* and *q* iterables together and sum the products.
I'm not sure but I suspect the original command is more performant. I'm fine also with adding the proposed item as an alternative illustration.
I would leave the .c file unchanged using map and operator.mul.
Sorry, something went wrong.
There was a problem hiding this comment.
ISTM the proposed extra sentence just repeats the first sentence before the rough equivalent.
Sorry, something went wrong.
|
I'm disinclined to accept this edit because it doesn't look nice. It adds tuple unpacking, introduces two new variables, and forces the reader to relate the px and py back to the inputs p and q. None of that is hard, but then the original wasn't hard either. Also, the original lived for a long time in the itertools docs and no one ever reported difficulty understanding it. So, I don't think there is a real problem to be solved here. Also, I like the current version because the words in the rough equivalent match the words in the preceding text: sum() Return the sum of values mul() of products map(..., p, q) from two iterables *p* and *q*. Also ISTM that the function being described is simple to grasp. When people see sumprod() in Excel, the name itself suffices for an explanation. If this were a more challenging, we could break it down further: def sumprod(p, q):
total = 0
for x, y in zip(p, q, strict=True):
total += x * y
return total
But I think that would insult the intelligence of our readers and not raise them up from where they began. Looking back at the thread, no one really supports this edit. skirpichev doesn't think it is a good idea. carol says to leave as-is will possible further elaboration. picnixz is neutral. And I'm against it as well. So marking as closed and thanking everyone for looking at this. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Docs-only change. Simplifies math.sumprod documentation so that it doesn't assume the reader knows about operator.mul, making it clear what kind of comprehensions can be replaced with math.sumprod().
📚 Documentation preview 📚: https://cpython-previews--130206.org.readthedocs.build/