-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PEP 769: Add a 'default' keyword argument to 'attrgetter' and 'itemgetter' #4179
base: main
Are you sure you want to change the base?
Conversation
Hello!
You can use 790 if you like, but we normally pick the next one in sequence, which currently would be 769. Would you prefer to use 769? I've also updated the PR description to include the new PEP checklist, please could you work down it and check them off as appropriate? Thanks! |
Please could you also wrap lines to 79 columns? https://peps.python.org/pep-0012/#general If you have prettier installed, this can do it: prettier --prose-wrap=always --print-width=79 -w filename.rst |
Hello @hugovk ! Thanks for the help and info.
I'm working now in making all linters go green. Thanks again for all the help! |
Linters and builders fixed! |
You're welcome!
Let's go for 769 then. If you make a new branch, you'll need to make a new PR. But the branch name isn't too important and the branch will be deleted after merge, so I'd just keep the current one. |
Cool, thanks! I renamed it to 769, then. Also updated PR title and description. Let me know when this is ready for the discussion in the forum, and I'll open a thread there. Thanks! |
.github/CODEOWNERS
Outdated
@@ -650,6 +650,7 @@ peps/pep-0768.rst @pablogsal | |||
peps/pep-0777.rst @warsaw | |||
# ... | |||
peps/pep-0789.rst @njsmith | |||
peps/pep-0790.rst @facundobatista |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And move this up a few lines:
peps/pep-0790.rst @facundobatista | |
peps/pep-0767.rst @facundobatista |
peps/pep-0769.rst
Outdated
except (TypeError, IndexError, KeyError): | ||
value = XYZ | ||
|
||
However, this would be not as eficient as we'd want for particular cases, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo:
However, this would be not as eficient as we'd want for particular cases, | |
However, this would be not as efficient as we'd want for particular cases, |
peps/pep-0769.rst
Outdated
About Posible Implementations | ||
----------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About Posible Implementations | |
----------------------------- | |
About Possible Implementations | |
------------------------------ |
peps/pep-0769.rst
Outdated
retrieve the item with a default. See examples in the About Posible | ||
Implementations subsection before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retrieve the item with a default. See examples in the About Posible | |
Implementations subsection before. | |
retrieve the item with a default. See examples in the About Possible | |
Implementations subsection above. |
Or we could add a reference?
For example, add this above:
+.. _PEP 769 About Possible Implementations:
About Posible Implementations
-----------------------------
And then:
retrieve the item with a default. See examples in the About Posible | |
Implementations subsection before. | |
retrieve the item with a default. See examples in the `About Possible | |
Implementations <PEP 769 About Possible Implementations_>`__ subsection above. |
peps/pep-0769.rst
Outdated
individual default values to multiple attributes or items. While some | ||
discussions considered allowing multiple defaults, the increased | ||
complexity and potential for confusion led to favoring a single default | ||
value for all cases (more about this below in Rejected Ideas). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add similar reference here if we want to hyperlink it.
peps/pep-0769.rst
Outdated
|
||
For the case of ``attrgetter`` is quite direct: it implies using | ||
``getattr`` catching a possible ``AttributeError``. So | ||
``attrgetter("name", default=XYZ)(obj)`` would be like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the following code is shown as code, not blockquote:
``attrgetter("name", default=XYZ)(obj)`` would be like: | |
``attrgetter("name", default=XYZ)(obj)`` would be like:: |
peps/pep-0769.rst
Outdated
value = XYZ | ||
|
||
Better performance, more complicated to implement and explain. This is | ||
the first case in the Open Issues section later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be a linked reference.
peps/pep-0769.rst
Outdated
except (TypeError, IndexError, KeyError): | ||
resutl = default | ||
|
||
(however see previous open issue about special case for dictionaries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(however see previous open issue about special case for dictionaries) | |
(However see previous open issue about special case for dictionaries.) |
Thanks! There I followed all your suggestions. I'm not 100% sure if the linked sections is fine, Github is not rendering those correctly but I understand that it may be a Github thing not supporting ReST 100% ok. Thanks again! |
Selected number: 769
This PEP is about adding a
default
keyword argument toattrgetter
anditemgetter
(from theoperator
module`).Thanks!
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approvalAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
📚 Documentation preview 📚: https://pep-previews--4179.org.readthedocs.build/pep-0769/