[issue17720] pickle.py's load_appends should call append() on objects other than lists

2017-01-26 Thread Serhiy Storchaka 

Serhiy Storchaka added the comment:

> From PEP 7:

Sorry, I meant PEP 307 of course.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2017-01-25 Thread Serhiy Storchaka 

Serhiy Storchaka added the comment:

Thanks. Opened issue29368 for this.

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2017-01-24 Thread Alexandre Vassalotti 

Alexandre Vassalotti added the comment:

I don't recall where I picked up the referred spec. Looking at PEP 307 and 
pickle docs [1], it does look like that objects which provides the listitems 
field of the reduce tuple should support extend() too.

It might best to try both in the implementation to avoid breaking existing code 
that relies on the previous behavior.

[1]: https://docs.python.org/3/library/pickle.html#object.__reduce__

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2017-01-22 Thread Serhiy Storchaka 

Serhiy Storchaka added the comment:

>From PEP 7:

listitemsOptional, and new in this PEP.
 If this is not None, it should be an iterator (not a
 sequence!) yielding successive list items.  These list
 items will be pickled, and appended to the object using
 either obj.append(item) or obj.extend(list_of_items).
 This is primarily used for list subclasses, but may
 be used by other classes as long as they have append()
 and extend() methods with the appropriate signature.
 (Whether append() or extend() is used depends on which
 pickle protocol version is used as well as the number
 of items to append, so both must be supported.)

Both append() or extend() must be supported, therefore old code was correct. C 
implementation can be optimized by using extend().

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2016-03-04 Thread Serhiy Storchaka 

Serhiy Storchaka added the comment:

Why this patch was not applied to 2.7? What is the spec of APPENDS?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-20 Thread Roundup Robot 

Roundup Robot added the comment:

New changeset 37139694aed0 by Alexandre Vassalotti in branch '3.3':
Isuse #17720: Fix APPENDS handling in the Python implementation of Unpickler
http://hg.python.org/cpython/rev/37139694aed0

--
nosy: +python-dev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-20 Thread Alexandre Vassalotti 

Changes by Alexandre Vassalotti alexan...@peadrop.com:


--
resolution:  - fixed
stage: patch review - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-14 Thread Alexandre Vassalotti 

Alexandre Vassalotti added the comment:

Alright alright! Here's a less bogus patch. :)

--
stage: needs patch - patch review
Added file: http://bugs.python.org/file29846/fix_loads_appends.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-13 Thread Alexandre Vassalotti 

New submission from Alexandre Vassalotti:

In pickle.py, load_appends is currently defined as

def load_appends(self):
stack = self.stack
mark = self.marker()
list = stack[mark - 1]
list.extend(stack[mark + 1:])
del stack[mark:]

However, according to the spec of APPENDS, it should actually be:

def load_appends(self):
obj = stack[mark - 1]
items = stack[mark + 1:]
if isinstance(obj, list):
obj.extend(items)
else:
for item in items:
obj.append(item)

This will match with the current behaviour of _pickle.

--
assignee: alexandre.vassalotti
components: Library (Lib)
files: loads_appends.patch
keywords: patch
messages: 186774
nosy: alexandre.vassalotti, pitrou, serhiy.storchaka
priority: normal
severity: normal
stage: needs patch
status: open
title: pickle.py's load_appends should call append() on objects other than lists
type: behavior
versions: Python 3.3, Python 3.4, Python 3.5
Added file: http://bugs.python.org/file29810/loads_appends.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-13 Thread Antoine Pitrou 

Antoine Pitrou added the comment:

Your patch is bogus (duplicated load_appends definition).
Also, I think you want to add a test if you want to avoid further regressions.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue17720] pickle.py's load_appends should call append() on objects other than lists

2013-04-13 Thread Serhiy Storchaka 

Serhiy Storchaka added the comment:

Yes, test is needed. Current tests don't cover this case.

I suggest to move obj.append out of the tight loop, as in C implementation.

append = obj.append
for item in items:
append(item)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17720
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com