classification
Title: add weakref support to types.SimpleNamespace
Type: enhancement Stage: patch review
Components: Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: amaury.forgeotdarc, christian.heimes, eric.snow, pitrou, rhettinger, sbt
Priority: normal Keywords: patch

Created on 2012-06-05 02:58 by eric.snow, last changed 2013-06-25 05:16 by eric.snow.

Files
File name Uploaded Description Edit
issue15004.diff eric.snow, 2012-06-05 05:24 change to type and added test review
simplenamespace-weakref.diff eric.snow, 2013-02-20 04:26 review
Messages (10)
msg162323 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-06-05 02:58
Currently types.SimpleNamespace does not support weak references.  Amaury asked for this in #14673, but I'd like to see more discussion.  What are the use cases for weak references in this case?

The type is a simple wrapper around dict, which does not support weak references.  In fact most of the builtin types do not support them.  However, I freely admit that I haven't used weak references very much and am not familiar with the use cases.
msg162326 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2012-06-05 04:39
The only reason some of the builtin types don't support weakrefs is to save space.  That reason doesn't apply here.  Most types should support weakrefs unless there is a compelling reason not to.
msg162328 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-06-05 05:24
That's good enough for me. :)
msg176876 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-12-04 07:13
The patch LGTM except for the extra code reformatting. However it's too late for 3.3.
msg176877 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-12-04 07:16
You shouldn't have to export something named __weakref__.
Furthermore, the test should check that weakrefs work, not that an attribute exists.
msg182253 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-17 02:08
I've yanked the __weakref__ attr--apparently builtins don't have them.  I've also update the test.  Is the updated test sufficient?  I expect that if weakref.ref(ns) works, the rest of the weakref functionality works as well.
msg182266 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-17 11:47
No, you should also test the weakref returns None once the object is garbage collected.
msg182467 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-20 04:26
Good point.  Thanks.  I've updated the test.  Does that cover it?
msg182492 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-20 12:11
Good, except that you have to add a gc.collect() call for the non-refcounted implementations.
msg182503 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-02-20 13:39
> Good, except that you have to add a gc.collect() call for the 
> non-refcounted implementations.

Better to use test.support.gc_collect().
History
Date User Action Args
2013-06-25 05:16:50eric.snowsetassignee: eric.snow
2013-02-20 13:39:04sbtsetnosy: + sbt
messages: + msg182503
2013-02-20 12:11:08pitrousetmessages: + msg182492
2013-02-20 04:26:39eric.snowsetfiles: - simplenamespace-weakref.diff
2013-02-20 04:26:23eric.snowsetfiles: + simplenamespace-weakref.diff

messages: + msg182467
2013-02-17 11:47:49pitrousetmessages: + msg182266
2013-02-17 02:08:18eric.snowsetfiles: + simplenamespace-weakref.diff

messages: + msg182253
2012-12-04 07:16:19pitrousetnosy: + pitrou
messages: + msg176877
2012-12-04 07:13:47christian.heimessetnosy: + christian.heimes

messages: + msg176876
versions: + Python 3.4, - Python 3.3
2012-06-07 07:37:01eric.snowsetstage: needs patch -> patch review
2012-06-05 05:24:24eric.snowsetfiles: + issue15004.diff
keywords: + patch
messages: + msg162328
2012-06-05 04:39:18rhettingersetnosy: + rhettinger
messages: + msg162326
2012-06-05 02:58:51eric.snowcreate