Most Set methods of KeysView and ItemsView do not work right

[stutzbach]

BPO	9214
Nosy	@rhettinger, @merwok, @durban
Files	keysview_test.py: Python 3 script to illustrate the problem issue9214.1.patch issue9214.2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/rhettinger'
closed_at = <Date 2010-08-22.08:02:24.830>
created_at = <Date 2010-07-09.19:31:43.889>
labels = ['easy', 'type-bug', 'library']
title = 'Most Set methods of KeysView and ItemsView do not work right'
updated_at = <Date 2010-08-22.08:02:24.828>
user = 'https://bugs.python.org/stutzbach'

bugs.python.org fields:

activity = <Date 2010-08-22.08:02:24.828>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2010-08-22.08:02:24.830>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2010-07-09.19:31:43.889>
creator = 'stutzbach'
dependencies = []
files = ['17923', '18208', '18267']
hgrepos = []
issue_num = 9214
keywords = ['patch', 'easy']
message_count = 10.0
messages = ['109788', '109789', '109790', '109805', '111593', '111990', '112046', '112117', '114459', '114641']
nosy_count = 5.0
nosy_names = ['rhettinger', 'stutzbach', 'eric.araujo', 'eli.bendersky', 'daniel.urban']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9214'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

[stutzbach]

Attached is a simple Python 3 example script that defines a minimalist MutableMapping that simply wraps a dict, followed by:

x = MySimpleMapping(red=5)
y = x.keys()
z = x.keys() | {'orange'}                                          x['blue'] = 7
print(list(z))
print(list(z))

Output:
['blue', 'red', 'orange']
[]

Expected Output:
['orange', 'red']
['orange', 'red']

The problem is that __or__ ends up returning a new KeysView wrapping a generator instead of returning a set.

To solve the problem, KeysView and ItemsView need to override _from_iterable to return a set instead of a new view.

[stutzbach]

Oops. Somehow deleted a linefeed in there. The example should read:

x = MySimpleMapping(red=5)
y = x.keys()
z = x.keys() | {'orange'}                                                       x['blue'] = 7
print(list(z))
print(list(z))

[stutzbach]

I guess roundup is deleting my linefeed? I'm sure I didn't do it that time. I'm not sure what's going on there, but the "x['blue'] = 7" should be on a line by itself.

[rhettinger]

To solve the problem, KeysView and ItemsView need
to override _from_iterable to return a set instead
of a new view.

Thanks for the good analysis and suggested fix. I believe both are correct and they match the behavior of real dictionaries.

  def _from_iterable(self, it):
     return set(it)

Proposed tests to match real dicts:

  x = MySimpleMapping()   # from stuzback's example
  x['red'] = 5
  y = x.keys()
  assert isinstance(y, collections.Set)
  assert not isinstance(y, collections.MutableSet)
  z = x.keys() | {'orange'}
  assert type(z) is set

[elibendersky]

Hello,

I'm attaching a patch for this issue.

1. _from_iterable in KeysView and ItemsView overridden as per Daniel's suggestion (Lib/_abcoll.py)
2. Added a test case to Lib/test/test_collections.py that uses this test case (creates the subclass and attempts modifying the view), with Raymond's suggestions incorporated, as well as a simple correctness test.

The patch was made against the latest dev checkout. If it's found sufficient, I can backport it to 2.6/2.7

---

P.S. The patch was generated from a Hg mirror, so to patch it -p1 is to be used. If this is a problem, let me know and I'll generate a patch from an SVN working directory. I got the impression from the #python-dev IRC channel that submitting patches from Hg is fine, but I may be wrong here.

[durban]

The patch applies cleanly to the py3k branch (r83238). The unittests pass.
The code looks good to me (it does exactly what was described as the solution). There are some trailing whitespace on some lines, that will need to be deleted.

[elibendersky]

Attaching an updated patch, with the trailing whitespace removed. Hope it's more acceptable now.

P.S. Please let me know how to detect such issues in future patches.

[durban]

P.S. Please let me know how to detect such issues in future patches.

In some editors there is an option "Remove trailing spaces" or something like that. Also, some editors (for example Kate) show trailing spaces.

[stutzbach]

The patch looks good to me, too. The new tests fail without the fix, and pass with the fix.

[rhettinger]

Fixed in r84252, r84252, and r84254.