-
-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add list.clear() and list.copy() #54725
Comments
Add list.clear() method with obvious semantics. Pro:
Anti:
Guido: (python-ideas list, 'Set Syntax' thread, today) |
Guido's email is archived at: |
Guido approved these both in a thread earlier this year. The reasoning for copy() was the same as for clear(), some folks couldn't cope with: b = a[:] |
Objects/listobject.c has a static function named list_clear used internally. Is it possible to just expose this function as a clear() method? One problem is that it has this strange comment in the end:
However, looking at the code I'm not sure the list can be cleared any more than the function does, and it actually deallocates the ob_item field of the list. |
Hi, I'm also looking at listobject.c also... if we want list.clear() to behave exactly like del list[], we may be able to just call list_ass_slice on the list. Similarly for list.copy which should behave like a=l[:] |
Note that when executed to do 'del lst[:]' (i.e. with argument v set to 0 |
Yes, list_clear should be called, but no, it cannot be used directly because a method needs a PyObject* return value. So a wrapper method is needed that looks like listappend() does for list.append(). list_copy() will just look like list_slice() with the index fiddling removed. |
That's good if it's so... can you explain why list_clear doesn't guarantee that the list is empty? Why would XDECREF populate the list? I don't quite understand it. eli: are you writing a patch for this? |
Attaching a patch for list.clear():
If this look fine to the more experienced devs, things left to do are:
Some random observations:
help({}.pop) gives: pop(...)
D.pop(k[,d]) -> v, remove specified key and return the corresponding value.
If key is not found, d is returned if given, otherwise KeyError is raised While help([].pop) gives: pop(...)
L.pop([index]) -> item -- remove and return item at index (default last).
Raises IndexError if list is empty or index is out of range. Note the '--' which separates the signature from description in the list version. |
Was list.copy() also approved? After all, there are many ways to achieve the same even now:
Especially re the last one: list.copy() can be deep or shallow, which one should it be? |
Also, where is the *official* place to document list objects and their methods? |
Yes, list.copy was also approved IIRC. And it should be a shallow copy, like all other copy methods on builtins. |
This is really welcome. It makes Python even more readable. If 'a' is a list object, a[:] is not so obvious at first to a newcomer, but Also, a.clear() is so perfect and understandable. I wish you could decorate Python versions prior to 3.3 with this two new list methods. |
Attaching a patch with the following:
Re the documentation, it's currently unclear where it should go. I asked on docs@python.org. |
Hi Eli, I think the right place is 4.6.4, It starts with “List and bytearray objects support additional operations For methods not supported by bytearray, you can use the fake footnote Regards |
Following Éric's suggestion, I'm attaching an updated patch with with the documentation in Doc/library/stdtypes.rst updated with the new methods. There seems to be a slight Sphinx markup problem with this addition. I rewrote note (8) as: :meth:`clear`, :meth:`copy` and :meth:`sort` are not supported by Unfortunately, :meth:`copy` is getting linked to the copy module. |
Nothing will happen on this until 3.2 is done and the py3k branch starts with 3.3 submissions. |
Eli, I learned this trick recently: :meth:`!copy`. |
Éric - thanks, it works [attaching updated patch]. However, don't you think the core problem is a Sphinx bug we should report? Raymond - this happens after final 3.2 release (on Feb 05 2011 if it's on schedule), right? |
I'm troubled with one little letter: "L.copy() -> list -- a shallow copy of L"); should be "L.copy() -> list -- shallow copy of L"); without the letter 'a', Please fix this. |
Can you please help me find the definition of the copy() method of dict in |
Boštjan, "a shallow copy": I took this directly from the documentation of dicts, which says: "D.copy() -> a shallow copy of D") As I mentioned in an earlier message, the doc-strings of list and dict methods are inconsistent in more than one way, so I'm going to leave this decision to the committer. I'll be happy to help with fixes too. Re your other question, in the Python source root, dictionaries are mostly implemented in Objects/dictobject.c - there's an array called mapp_methods that lists the functions used to implement relevant methods. For copy() it lists:
So you need dict_copy. Note that it's just a wrapper (of another wrapper, by the way) bit it's a good place to start. Arm yourself with an editor or IDE with some code-searching capabilities. |
mapp_methods ? Don't you mean map_methods ? |
No, he means mapp_methods. Why don't you simply look at the file? |
Please modify the patch so that it can be applied to current py3k trunk cleanly. (Notice that Lib/collections.py has been changed to a package in bpo-11085). |
Ray: Eli can just refresh his patch and commit. Note that the patch program will prompt you for a file name if it can’t find the file for a diff hunk, so it should be trivial to apply a patch across a rename. Eli: Would you mind changing two nits before committing the nice patch? +"L.clear() -> None -- remove all items from L"); + + |
Eli doesn't need to post a new patch. I'm sure he will fix any nits in his commit. |
On Thu, Feb 24, 2011 at 05:26, Éric Araujo <report@bugs.python.org> wrote:
I will fix and commit over the weekend. |
[Éric Araujo]
These kind of nitty comments really aren't helpful. PyDoc_STRVAR(clear__doc__, Just because list.remove.__doc__ failed to consistently follow that convention doesn't make Eli's patch incorrect. |
A slightly revised patch committed in revision 88554:
Éric - as I mentioned earlier in this issue, I chose to leave the syntax of the docstring for the new methods similar to the same methods in dict (dict docstring look better and more internally consistent anyhow). I propose to move further discussion of this matter into a separate issue which will deal with the overall (in)consistency in the docstrings of list and dict. |
Reading "clear and copy are not supported by bytearray": shouldn't they be? ("sort" probably really makes no sense on bytearrays.) |
On Fri, Feb 25, 2011 at 10:11, Georg Brandl <report@bugs.python.org> wrote:
Perhaps they should, and it's not a big deal to implement. But I'm not |
Unless someone raises a controversial and non-trivial issue about adding clear() and copy() to bytearray, there is no need for a python-dev discussion on the subject. Just post a patch to the tracker. |
Yes, it should be discussed on python-dev. |
In any case, this issue can be closed. |
I have installed Python 3.2 final on my Windows machine and I get an |
Because they got added *after* 3.2 was released? |
Right, right. My bad. Can't wait for Python 3.3! ;) |
Georg, what is the issue? Is there some reason that bytearrays should not be copied or cleared? Is there some reason to prefer the current: dup = b[:] # copy |
No -- but the question is if copy() and clear() mightn't be added to the (mutable) sequence ABC if we make all builtin such sequences implement them. |
The ABCs are subset of the methods for the concrete APIs. We've avoided the likes of copy() because it requires knowledge of the constructor's signature -- for example, MutableMapping does not cover copy(). It is okay for Eli to add MutableSequence.clear() because it can be implemented in terms of pop(), much like we do for MutableMapping.clear(). Eli, feel free to create a patch to add clear() and copy() to bytearray and to add clear() to MutableSequence. Assign the patch to me for review. |
Attaching a patch adding copy() and clear() to bytearrays, with tests and doc. I didn't add the methods to MutableSequence because I have a doubt about it - in particular which exception get raised by .pop if it's empty. Curiously, lists and bytearrays raise different exceptions in this case - IndexError and OverflowError, respectively. |
The patch is fine. Do consider using assertIsNot() in the tests. Then go ahead and apply it. The OverflowError in bytearray.pop() is a bug, please open a separate report for it and make a patch changing it to IndexError. Assign that bug report to me. Go ahead and propose a patch for MutableSequence.clear() implemented with MutableSequence.pop() and catching an IndexError when empty. Thanks for your efforts. |
Hmm, shouldn't self.__class__(self) be a good default implementation of copy()? I'd expect any sequence to support this way of creation from another sequence, even if it's inefficient. |
The copy() method isn't being proposed for MutableSequence because it presumes that we know something about the constructor's signature. For example, the constructor of array() needs the element storage type as an argument. We refuse the temptation to guess. In the Set api, we had no choice because many set-methods necessarily create a new set. To handle the constructor signature problem, the creation step was factored-out into the from_iterable() method so that a user could override it if necessary. Also copy() is handled oddly in the builtin types. To handle the constructor signature issue for subclasses, they ignore the subclass and return a instance of the base class. For example, the inherited copy() method on a subclass of list or dict will create an instance of list or dict, not of the subclass itself. Accordingly, subclasses that want instances of themselves have to override the inherited copy() method. They would have to do this anyway if they subclass contained any other data in the class dictionary that would need to be passed along to a copy. In short, we're better-off not supplying copy() as part of the MutableSequence ABC. |
Committed the bytearray methods in revision 88733. Closing this issue. Will handle wrong exception and MutableSequence ABC method addition in separate issues. |
New changeset 958a98bf924e by Eli Bendersky in branch 'default': |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: