Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(7541)

Issue 8882: Slicing support in Python

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 5 months ago by Alek Storm
Modified:
14 years, 9 months ago
Reviewers:
kenton, pesho.petrov
Base URL:
http://protobuf.googlecode.com/svn/trunk/
Visibility:
Public.

Description

This patch (attached, works with r81) adds slicing support for repeated fields in Python. It implements retrieval, setting, and deletion for both scalar and composite repeated fields, and adds several unit tests for each method. There was already a remove() method for repeated scalars, but not for composites. I added one without any difficulty. Did I miss something? In fact, this whole patch was pretty easy. There must be some reason it hasn't been done before. The __eq__() method for repeated composites disallows comparing to any other sequence type, including ordinary lists. Why can't we do this? Adding more methods to the _RepeatedScalarFieldContainer and _RepeatedCompositeFieldContainer classes makes it even more painfully obvious that they need to be merged. I'm sure we can make it polymorphic, and that'll probably be my next patch.

Patch Set 1 #

Patch Set 2 : Trying again, from a different directory #

Patch Set 3 : Okay, *now* it's working. Go ahead. #

Patch Set 4 : Removed __setslice__ method for composites #

Patch Set 5 : Okay, removed all assignment methods and remove() for composites #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -26 lines) Patch
python/google/protobuf/internal/containers.py View 2 3 4 5 chunks +41 lines, -10 lines 0 comments Download
python/google/protobuf/internal/reflection_test.py View 2 3 4 5 chunks +56 lines, -16 lines 1 comment Download

Messages

Total messages: 8
Alek Storm
15 years, 5 months ago (2008-12-02 20:32:27 UTC) #1
Alek Storm
Huh, the side-by-side diffs feature doesn't work. The repo doesn't have an 'svn-history' directory.
15 years, 5 months ago (2008-12-02 20:34:57 UTC) #2
kenton
Strange, many people have used this successfully before...
15 years, 5 months ago (2008-12-02 21:05:50 UTC) #3
Alek Storm
Okay, *now* it's working. Go ahead.
15 years, 5 months ago (2008-12-03 00:44:56 UTC) #4
kenton
On 2008/12/03 00:44:56, Alek Storm wrote: > Okay, *now* it's working. Go ahead. Cool. Petar, ...
15 years, 5 months ago (2008-12-03 01:16:12 UTC) #5
Alek Storm
Removed __setslice__ method for composites
15 years, 4 months ago (2008-12-18 16:32:59 UTC) #6
Alek Storm
Okay, removed all assignment methods and remove() for composites
15 years, 4 months ago (2008-12-20 03:59:57 UTC) #7
pesho.petrov
15 years, 4 months ago (2008-12-20 04:19:38 UTC) #8
Looks Good!

http://codereview.appspot.com/8882/diff/605/403
File python/google/protobuf/internal/reflection_test.py (right):

http://codereview.appspot.com/8882/diff/605/403#newcode59
Line 59: class ReflectionTest(unittest.TestCase):
Thanks! I never noticed this typo.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b