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

Setting default multiconstructor results in an TypeError #317

Closed
Losik opened this issue Jul 3, 2019 · 3 comments
Closed

Setting default multiconstructor results in an TypeError #317

Losik opened this issue Jul 3, 2019 · 3 comments

Comments

@Losik
Copy link

Losik commented Jul 3, 2019

The issue is about the following lines in construct_object method of BaseConstructor class

for tag_prefix in self.yaml_multi_constructors:
if node.tag.startswith(tag_prefix):
tag_suffix = node.tag[len(tag_prefix):]
constructor = self.yaml_multi_constructors[tag_prefix]
break
else:
if None in self.yaml_multi_constructors:
tag_suffix = node.tag
constructor = self.yaml_multi_constructors[None]

I wanted to set up a multiconstructor for unknows tags. It seems like these lines are responsible for picking a default multiconstructor and therefore to define one I had to add a multiconstructor for None value as a tag_prefix

if None in self.yaml_multi_constructors:
tag_suffix = node.tag
constructor = self.yaml_multi_constructors[None]

At the very least the lines hint that None is considered a valid key for self.yaml_multi_constructors. But the only way the execution can reach these lines is by traversing all the keys in self.yaml_multi_constructors first.

Now, if None is one of the keys, we get an exception TypeError: startswith first arg must be str, unicode, or tuple, not NoneType on line 77 making lines 83-84 effectively unreachable

if node.tag.startswith(tag_prefix):

Minimal non-working example:

import yaml

class MyLoader(yaml.loader.Loader):
    pass

def default_multi_constructor(loader, tag_suffix, node):
    return node.tag

MyLoader.add_multi_constructor(None, default_multi_constructor)

if __name__ == '__main__':
    print yaml.load('''!som:multitag {}''', MyLoader)
@perlpunk
Copy link
Member

Maybe this line
if None in self.yaml_multi_constructors
was intended to support multi_constructors the way you try to define it, but it was never tested.

But another way to add a multi constructor for unknown tags is:

# catch all tags with a single !
MyLoader.add_multi_constructor('!', default_multi_constructor)
# also catch unknown !! tags
MyLoader.add_multi_constructor('tag:yaml.org,2002:', default_multi_constructor)

So you can use this for now.

But if None was intended to do this, then we probably can make it work again:

If I change this line
if node.tag.startswith(tag_prefix):
to this:
if tag_prefix is not None and node.tag.startswith(tag_prefix):

then your code
MyLoader.add_multi_constructor(None, default_multi_constructor)
is working for all tags.
All tests are still passing.

@perlpunk perlpunk added this to Backlog in 5.3 Release Dec 2, 2019
perlpunk added a commit that referenced this issue Dec 6, 2019
Loader.add_multi_constructor(None, myconstructor)

Also add test for add_multi_constructor('!', ...) etc.

See issue #317
perlpunk added a commit that referenced this issue Dec 6, 2019
Loader.add_multi_constructor(None, myconstructor)

Also add test for add_multi_constructor('!', ...) etc.

See issue #317
@perlpunk
Copy link
Member

perlpunk commented Dec 6, 2019

I created #358 which will hopefully released soon with 5.3

perlpunk added a commit that referenced this issue Dec 7, 2019
Loader.add_multi_constructor(None, myconstructor)

Also add test for add_multi_constructor('!', ...) etc.

See issue #317
@perlpunk perlpunk moved this from Backlog to Done in 5.3 Release Dec 7, 2019
@perlpunk
Copy link
Member

perlpunk commented Jan 6, 2020

released https://pypi.org/project/PyYAML/5.3/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

2 participants