| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
`d1 + d2` is semantically identical to `d3 = d1.copy(); d3.update(d2); d3`, and `d1 += d2` is semantically identical to `d1.update(d2)`. Isn't that sweet?
It feels so weird to sum dicts like this... cool.
This gets the collections tests passing again.
This adds one test with 5 new assertions.
In case this gets pulled. Very cool!
Whoops.
This whole patch adds 3 new objects with docstrings: dict.__add__, dict.__radd__, dict.__iadd__. On some builds, this puts us over the threshold, so bump the count!
|
It usually isn't an effective use to time to go straight to a PR before a proposal has been discussed and accepted. |
Sorry, something went wrong.
|
I figured it would help with decision-making. The person who started the thread asked for an implementation, and I had the free time to make one. I'm not very familiar with how the PEP process usually works, but I saw in PEP 1:
Sorry if this is a distraction. It's not my intention to disrupt the normal process! |
Sorry, something went wrong.
Replaces two places where a failed PyDict_Update call returns NotImplemented instead of raising the error.
|
It looks like this may yet have a chance :-) |
Sorry, something went wrong.
This brings this reference implementation in line with PEP 584's spec. Separately, allows RHS of += to be any valid arg to dict.update.
These are implemented as they are currently laid out in PEP 584.
This should get tests passing again.
And... we're officially implemented!
This fixes issues where iteration errors could be swallowed, and also stops a possible reference leak.
This also implements the complex keys/__iter__ dance that update uses, as an example of compatibility with dict.update.
dict.__iadd__ can take any mapping (i.e., a list).
This is pretty much copied-and-pasted from UserDict, but it's better than what we had before. They may need some tweaking.
This was requested by Steven for comparison purposes. The final implementation will only keep one!
|
Because list and set do not call copy() and extend()/update(). It would make dict very different from other builtin types. |
Sorry, something went wrong.
|
I agree with Serhiy.
On Thu, Feb 6, 2020 at 00:07 Serhiy Storchaka ***@***.***> wrote:
Because list and set do not call copy() and extend()/update().
It would make dict very different from other builtin types.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12088?email_source=notifications&email_token=AAWCWMUS4RJZZ5BMOQH224DRBPAMNA5CNFSM4G2X5HR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK6JJ6Q#issuecomment-582784250>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWCWMULOM22F7PTNNNJGM3RBPAMNANCNFSM4G2X5HRQ>
.
--
--Guido (mobile)
|
Sorry, something went wrong.
|
Since it is a principal point of the PEP, I suggest you to create two or more separate PRs: one does not depend on subclasses, and the other calls copy() and update(). Yet one argument against calling copy() and update() is that it would be the only case when operator depends on non-dunder methods. |
Sorry, something went wrong.
|
@serhiy-storchaka Yes, and in the earlier stages of the proposal I felt as you did: But Steven was strongly for this, and over time his arguments were enough to shift my view. Also:
It's more common to subclass dict than either list or set, so I see this as more important issue than just blindly following what seems like a less-than-ideal precedent without a really good reason to. dict doesn't have any operators yet, so we're not bound by any prior design constraints in this area (rather, we're setting them). My example above with defaultdict shows that this has immediate benefits for both usability and maintenance. |
Sorry, something went wrong.
|
@gvanrossum, is this enough of an issue that you would consider rejecting the proposal over it? I know that Steven and I have somewhat strongly formed opinions on this from all of the endless mailing-list threads, so it might be a good idea to continue this discussion on the python-dev thread where he can participate, if so. |
Sorry, something went wrong.
|
Also, @serhiy-storchaka it would be pretty easy to just strip the last 5 commits at this point. I can make another PR if the two designs deviate more substantially. |
Sorry, something went wrong.
|
@brandtbucher This won't be an issue unless you make it one. The proposal is a good one regardless of what it does with subclasses. Does the PEP explicitly address this (either in the spec or in the text about rejected alternatives or elsewhere)? If so can you point me to the text? A general problem with honoring subclasses is that it's worse when it's done inconsistently than not at all. For immutable objects (e.g. Steven's int subclassing example) the problem is that you won't know the arguments to the subclass constructor, and you can't mutate int objects after they've been created. So you just have to suck it up. For dict, in this case copy()+update() will work. But that's just luck. C code that inserts a new key into a dict shouldn't have to look for an overridden __setitem__, it should be able to just use PyDict_SetItem. |
Sorry, something went wrong.
|
I don't believe this was ever explicitly outlined in the PEP (probably because we didn't get much pushback relative to other decisions 😉), but the Reference Implementation section is clear - it uses bound self methods rather than unbound dict methods. We also mention the desire of subclass-preserving behavior in the Motivation section, although that is less relevant because the operators could still be manually overridden. Thinking out loud, if dict were to grow more operators, they would likely be &, ^, and - (with the associated in-place versions, which is important). Continuing our subclass-friendliness started here would probably require, at the very least, also respecting __delitem__ and __setitem__ as well... How do we feel about keeping the copy call and dropping the update one? I know Steven was open to that. New subclasses just have to implement copy to get fast, working versions of everything, and we get to keep a working defaultdict, etc... |
Sorry, something went wrong.
|
I still think it's a doubtful choice (and the PEP is clear that the code there is not exactly equivalent). Can you bring this up in the python-dev thread? So far you've convinced 0 out of 2 core devs -- but you still could get the Steering Council behind you. |
Sorry, something went wrong.
|
Sorry, one out of three. :-)
|
Sorry, something went wrong.
|
Sorry, something went wrong.
|
This happened right?
|
Sorry, something went wrong.
|
Yes, I commented in the new PEP thread: |
Sorry, something went wrong.
|
I am going to go ahead and remove the update calls, since we've come to an agreement on that. Jury's still out on copy. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM. Obviously let's wait until PEP 584 is accepted.
Sorry, something went wrong.
|
Thanks for all of the feedback @gvanrossum and @serhiy-storchaka (especially for your insightful comments on the subclass issue). |
Sorry, something went wrong.
|
Guido just informed us that the Steering Council accepted his recommendation! |
Sorry, something went wrong.
|
@gvanrossum: Please replace # with GH- in the commit message next time. Thanks! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
https://bugs.python.org/issue36144