Skip to content

Commit

Permalink
Maps keys, items, and values return views, not generators
Browse files Browse the repository at this point in the history
Summary:
This diff is implementing `MapViews` for `py3` and fixing the failed unit test.

[Context Post](https://fb.workplace.com/groups/1730279463893632/permalink/3935447050043518/): Thrift-py3 `.values()`, `.items()`, `.keys()` return generator instead of a python view. This causes incompatibility issues for users upgrading to thrift-python. It also means some view apis like `len` don't work, and mean the return object is only once-iterable.

User Request: https://fb.workplace.com/groups/thrift.py3/permalink/1480673825975358/

#buildall

Reviewed By: yoney, ahilger

Differential Revision: D68674342

fbshipit-source-id: 09dcb6f79d04ed70b1eb70bb0b6898a70ed35105
  • Loading branch information
Danfeng Wang authored and facebook-github-bot committed Feb 3, 2025
1 parent 031c222 commit 214acc9
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ def test_isinstance(self) -> None:
self.assertNotIsInstance(StrIntMap(str_int_map), Map__Color_Color)
self.assertNotIsInstance(StrIntMap(str_int_map), Map__string_List__i32)

def test_map_views(self) -> None:
str_int_map = StrIntMap({"foo": 5})
self.assertEqual(len(str_int_map), 1)
self.assertEqual(len(str_int_map.keys()), 1)
self.assertEqual(len(str_int_map.values()), 1)
self.assertEqual(len(str_int_map.items()), 1)

def test_getitem(self) -> None:
x = StrStrMap({"test": "value"})
self.assertEqual(x["test"], "value")
Expand Down
13 changes: 3 additions & 10 deletions third-party/thrift/src/thrift/lib/py3/types.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -672,19 +672,12 @@ cdef class Map(Container):
yield from self._py_obj

def keys(Map self):
return self.__iter__()
return self._py_obj.keys()

def values(Map self):
if not self:
return
yield from self._py_obj.values()

return self._py_obj.values()
def items(Map self):
if not self:
return
yield from self._py_obj.items()


return self._py_obj.items()
CompiledEnum = _fbthrift_python_Enum
Enum = _fbthrift_python_Enum
# I wanted to call the base class Enum, but there is a cython bug
Expand Down

0 comments on commit 214acc9

Please sign in to comment.