classification
Title: C implementation of ElementTree: Some functions should support keyword arguments
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, cmn, eli.bendersky, ezio.melotti
Priority: release blocker Keywords: patch

Created on 2012-05-15 19:43 by cmn, last changed 2012-05-29 03:06 by eli.bendersky. This issue is now closed.

Files
File name Uploaded Description Edit
issue14818.diff ezio.melotti, 2012-05-17 16:19 review
issue14818-2.diff ezio.melotti, 2012-05-18 11:22 review
issue14818.3.patch eli.bendersky, 2012-05-28 03:55 review
Messages (11)
msg160753 - (view) Author: Markus (cmn) * Date: 2012-05-15 19:43
The C implementation of ElementTree which is the default by python3.3 [1] does not accept the namespaces keyword for Element/ElementTree.find(all|iter|...) and maybe others.


[1] http://bugs.python.org/issue13988
msg160756 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-05-15 20:02
As I wrote on the other issue:

> It seems to me that namespaces are actually supported, but they are
> accepted only as positional args and not keyword args, so this should 
> be easy to fix.

I can prepare a patch unless someone else is faster than me.
msg160861 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-05-16 13:43
Thanks for the report - such regressions are taken seriously and will be fixed.

Ezio - you can go ahead and prepare a patch. I'm currently away from home (business trip) but I will look into it when I get back next weak.
msg160976 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-05-17 16:19
The attached patch adds support for keyword args to find, findtext, findall, and iterfind.  iter and itertext don't seem to accept a namespace argument.  The patch includes minimal tests.

The name I used for the kwargs are the same of the Python version.  The documentation uses different names, and extra args (like "namespaces") are not even documented.

I didn't have time yet to double-check if other functions needs to be updated and to add better tests that actually check for specific namespaces.
msg161037 - (view) Author: Markus (cmn) * Date: 2012-05-18 06:44
Applied the patch, but could not verify 'it works for me' as Element lacks the attrib keyword.
msg161045 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-05-18 10:59
Attached patch fixes Element constructor to accept 'attrib' as keyword arg.  I couldn't find a way to make this work using PyArg_ParseTupleAndKeywords, so I ended up parsing the kwds by hand.
While adding more tests I found out another difference with the Python version.  The C version raises a TypeError if attrib is not a dict, whereas the Python version raises an AttributeError while attempting to do a .copy() of the object.  I changed the Python version to raise a TypeError too.
msg161080 - (view) Author: Markus (cmn) * Date: 2012-05-18 19:24
SubElement needs to handle the attrib keyword too.
msg161176 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-05-20 02:57
Ezio,

Your patch looks good, with these notes:

* When raising TypeError in the Python constructor, say that 'attrib' must be a dict, otherwise it's not clear *what* must be a dict.
* The 'attrib' keyword handling has to be added to SubElement too, as Markus said. I guess it would make sense to move it into some helper function to avoid duplication.
* The new keyword arguments have to be documented in the ReST docs of ET, since they haven't been documented until now
msg161756 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-05-28 03:44
The whole namespace issue is not very well documented in the ReST doc for ET - should open a new issue on this
msg161757 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-05-28 03:55
Updated patch with fixed error messages, additional tests and some documentation. No support in SubElement yet
msg161841 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-05-29 03:06
Committed all fixes in http://hg.python.org/cpython/rev/7d252dbfbee3
History
Date User Action Args
2012-05-29 03:06:15eli.benderskysetstatus: open -> closed
resolution: fixed
messages: + msg161841

stage: patch review -> resolved
2012-05-28 03:55:37eli.benderskysetfiles: + issue14818.3.patch

messages: + msg161757
2012-05-28 03:44:41eli.benderskysetmessages: + msg161756
2012-05-20 02:57:33eli.benderskysetmessages: + msg161176
2012-05-18 19:24:36cmnsetmessages: + msg161080
2012-05-18 11:22:51ezio.melottisetfiles: + issue14818-2.diff
2012-05-18 11:22:35ezio.melottisetfiles: - issue14818-2.diff
2012-05-18 10:59:52ezio.melottisetfiles: + issue14818-2.diff

messages: + msg161045
2012-05-18 06:44:11cmnsetmessages: + msg161037
2012-05-17 16:34:08Arfreversettitle: C implementation of ElementTree causes regressions -> C implementation of ElementTree: Some functions should support keyword arguments
2012-05-17 16:19:08ezio.melottisetfiles: + issue14818.diff
keywords: + patch
messages: + msg160976

stage: needs patch -> patch review
2012-05-16 13:43:43eli.benderskysetmessages: + msg160861
2012-05-15 20:40:59Arfreversetnosy: + Arfrever
2012-05-15 20:02:08ezio.melottisetpriority: normal -> release blocker

nosy: + ezio.melotti, eli.bendersky
messages: + msg160756

stage: needs patch
2012-05-15 19:43:37cmncreate