Opened 9 years ago
Closed 4 years ago
#5789 task closed fixed (fixed)
Remove usage of UserDict
Reported by: | Thijs Triemstra | Owned by: | Pawel |
---|---|---|---|
Priority: | lowest | Milestone: | Python-3.x |
Component: | core | Keywords: | |
Cc: | Thijs Triemstra | Branch: |
branches/userdict-removal-5789
branch-diff, diff-cov, branch-cov, buildbot |
Author: | adiroiban |
Description (last modified by )
Remove usage of UserDict
once Python 2.6 support is dropped completely (or manhole.explorer
or protocols.postfix
are ported to python 3).
twisted/protocols/postfix.py:import UserDict twisted/protocols/postfix.py: UserDict.UserDict): twisted/manhole/explorer.py:import UserDict twisted/manhole/explorer.py:class Pool(UserDict.UserDict):
Attachments (2)
Change History (13)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Summary: | Replace UserDict imports with collections version → Replace usage of UserDict with twisted.python.compat version |
Meh, not sure why I didnt notice that before. So in Python 2.7 it's in the old place and Python 3 moves it to the collections module. I suggest to add a twisted.python.compat version in that case like we did with execfile
.
comment:3 Changed 9 years ago by
In many cases, UserDict
is replaceable with dict
, which is better than importing some crufty compatibility code. I would suggest trying each case separately.
comment:4 Changed 9 years ago by
Keywords: | review py3k added |
---|---|
Summary: | Replace usage of UserDict with twisted.python.compat version → Remove usage of UserDict |
Attached patch takes care of UserDict, by inheriting directly from dict. It's a very small patch, but this is kinda low-level so smaller is better; I'm going to open a separate ticket for replacing DictMixin with MutableMapping (I almost have a patch, but it's a different change again and smaller is better). The changes here should be ok - all the touched lines are tested and all tests pass.
In lieu of what this ticket actually does, I'll change the summary to just "Remove usage of UserDict"
comment:5 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Vladimir Perić |
Thanks for working on this. Most changes are indeed covered by tests.
twisted.protocols.postfix
the docstring"""A dict that preserves insert order whenever possible."""
should be on 3 lines- Same for
"""A dict that preserves insert order whenever possible."""
intwisted.python.util
- It's missing a
.removal
newsfile - There are 3 lines not covered for
OrderedDict
and since you're changing it's class it would be good to have tests for those lines (copy
anditerkeys
) as well for peace of mind.
Can you put the patch in a branch as well so we can buildbot test it?
comment:6 Changed 9 years ago by
1, 2: will do 3: Why .removal? No features are getting removed here, I wouldn't give it more than a .misc personally. 4: Ah, I really don't feel like doing this, in part because those tests themselves probably also need cleanup and I don't feel like doing that. As it stands, all the changes *are* tested and the ticket does what it says it does. Still, if you want to insist on this, I guess I'll have to do it. :)
comment:7 Changed 8 years ago by
Description: | modified (diff) |
---|---|
Priority: | normal → lowest |
- during the python 3 port (r36719)
OrderedDict
inpython.util
changed it's base class fromUserDict
toobject
for python 3 only. - usage of
DictMixin
inhttp_headers
was removed during the python 3 port. - the following classes still use
UserDict
(excludingpython.util
):twisted/protocols/postfix.py:import UserDict twisted/protocols/postfix.py: UserDict.UserDict): twisted/manhole/explorer.py:import UserDict twisted/manhole/explorer.py:class Pool(UserDict.UserDict):
I suggest we remove usage of UserDict
once Python 2.6 support is dropped or python 3 support is added for manhole.explorer
or protocols.postfix
. Updating the ticket description and priority.
Changed 5 years ago by
Attachment: | userdict.patch.diff added |
---|
comment:8 Changed 5 years ago by
Keywords: | review added; py3k removed |
---|---|
Owner: | Vladimir Perić deleted |
comment:9 Changed 5 years ago by
Author: | → adiroiban |
---|---|
Branch: | → branches/userdict-removal-5789 |
(In [46232]) Branching to userdict-removal-5789.
comment:10 Changed 5 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Pawel |
Hi,
Many thanks for the patch!
We can not just remove True and False from twisted/manhole/explorer.py as they are public. If you want to get rid of them, they will need to go to a deprecation first.
I have reverted those changes.
Pool.data was a public member. We can not remove it without a deprecation as code using Pool() instanced as UserDict will break.
I have committed an example of deprecating the data
attribute.
Deprecation policy is explained here https://twistedmatrix.com/trac/wiki/CompatibilityPolicy
Here is a update of the policy with awaits review
https://github.com/twisted/twisted/compare/trunk...deprecation-docs-8082
I know that this is a minor change, but we made a compatibility promise to our users.
If you think that this need an exception, please ask for exception approval over the mailing list.
Please update the patch to deprecate the attributes removed together with the removal of UserDict and add tests for the deprecations.
Thanks!
comment:11 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
twisted.manhole is gone. UserDict is only used in twisted.protocols.postfix. On Python 3, it is imported from collections.
Note that UserDict isn't in the collections module in 2.X. Regarding DictMixin, you should be able to use MutableMapping with some adjustement.