Skip to content
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

bpo-39495: Remove default value from C impl of TreeBuilder.start #18275

Open
wants to merge 2 commits into
base: master
from

Conversation

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Jan 30, 2020

@hauntsaninja

This comment has been minimized.

Copy link
Contributor Author

hauntsaninja commented Jan 30, 2020

As suggested by Serhiy, this changes the C implementation of TreeBuilder.start to remove the default value.

Note however, that the C implementation of TreeBuilder.start still accepts None as a value whereas the Python implementation TypeErrors (see snippet at end).

I looked into changing the C implementation to also throw a TypeError. Unfortunately, it appears treebuilder_handle_start is called elsewhere where attrib can be Py_None (set on line 3310) and I'm not familiar enough with this module to know if I can break that:

res = treebuilder_handle_start((TreeBuilderObject*) self->target,

~/dev/cpython bpo39495 λ ./python.exe                       
Python 3.9.0a3+ (heads/bpo39495_2-dirty:2e6569b669, Jan 29 2020, 22:45:33) 
[Clang 11.0.0 (clang-1100.0.33.17)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import xml.etree.ElementTree
>>> # show the fix
>>> xml.etree.ElementTree.TreeBuilder().start("asdf")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: start expected 2 arguments, got 1
>>> # function still accepts None
>>> xml.etree.ElementTree.TreeBuilder().start("asdf", None)
<Element 'asdf' at 0x10b2e6710>

vs

~ λ python3.9
Python 3.9.0a3+ (heads/master:188bb5b, Jan 29 2020, 22:55:03) 
[Clang 11.0.0 (clang-1100.0.33.17)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import xml.etree.ElementTree
>>> # not fixed
>>> xml.etree.ElementTree.TreeBuilder().start("asdf")
<Element 'asdf' at 0x10b89f2c0>
>>> # function accepts None
>>> xml.etree.ElementTree.TreeBuilder().start("asdf", None)
<Element 'asdf' at 0x10b89f310>
>>> from test.support import import_fresh_module  
>>> pyElementTree = import_fresh_module('xml.etree.ElementTree', blocked=['_elementtree'])
>>> # show the inconsistency with python implementation
>>> pyElementTree.TreeBuilder().start("asdf")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: start() missing 1 required positional argument: 'attrs'
>>> # function does not accept None, throws TypeError
>>> pyElementTree.TreeBuilder().start("asdf", None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/shantanu/.pyenv/versions/3.9-dev/lib/python3.9/xml/etree/ElementTree.py", line 1463, in start
    self._last = elem = self._factory(tag, attrs)
  File "/Users/shantanu/.pyenv/versions/3.9-dev/lib/python3.9/xml/etree/ElementTree.py", line 171, in __init__
    raise TypeError("attrib must be dict, not %s" % (
TypeError: attrib must be dict, not NoneType
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.