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

fix yaml construct a slot object #258

Closed
wants to merge 2 commits into from

Conversation

Hanaasagi
Copy link

Env:

  • Python 3.7
  • pyyaml master branch

Test code:

import yaml


class A:
    __slots__ = ('c',)

    def __init__(self):
        self.c = 3


data = yaml.dump({'a': A()})
d = yaml.load(data)

Trackback:

Traceback (most recent call last):
  File "t.py", line 12, in <module>
    d = yaml.load(data)
  File "/root/py36/lib/python3.7/site-packages/yaml/__init__.py", line 72, in load
    return loader.get_single_data()
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 37, in get_single_data
    return self.construct_document(node)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 46, in construct_document
    for dummy in generator:
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 398, in construct_yaml_map
    value = self.construct_mapping(node)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 204, in construct_mapping
    return super().construct_mapping(node, deep=deep)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 129, in construct_mapping
    value = self.construct_object(value_node, deep=deep)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 88, in construct_object
    data = constructor(self, tag_suffix, node)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 617, in construct_python_object_new
    return self.construct_python_object_apply(suffix, node, newobj=True)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 608, in construct_python_object_apply
    self.set_python_instance_state(instance, state)
  File "/root/py36/lib/python3.7/site-packages/yaml/constructor.py", line 570, in set_python_instance_state
    setattr(object, key, value)
TypeError: can't set attributes of built-in/extension type 'object'

The exception was throwed from #L570

def set_python_instance_state(self, instance, state):
if hasattr(instance, '__setstate__'):
instance.__setstate__(state)
else:
slotstate = {}
if isinstance(state, tuple) and len(state) == 2:
state, slotstate = state
if hasattr(instance, '__dict__'):
instance.__dict__.update(state)
elif state:
slotstate.update(state)
for key, value in slotstate.items():
setattr(object, key, value)

I think object should be instance.

@Hanaasagi
Copy link
Author

resolves #232

@Hanaasagi
Copy link
Author

This PR has been open for a week. Can I get a review? @ingydotnet

@Hanaasagi
Copy link
Author

ping @perlpunk

@perlpunk
Copy link
Member

Sorry, I don't know enough python yet, so I don't know about slots. @nitzmahone maybe?

@MRigal
Copy link

MRigal commented May 3, 2019

Very valid fix and proper tests! However, it is just a minimal update to the already exising PR #161 which you could have mentioned.

@perlpunk This is definitely good code and a problem since a while!

@Hanaasagi
Copy link
Author

May I get a review @nitzmahone

Copy link

@paulgmiller paulgmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hit this same thing and made the same fix locally. Not sure if anyone will approve if this is full loader is deprecated though.

@perlpunk
Copy link
Member

perlpunk commented Dec 6, 2019

Thanks! I'll try to look at it this weekend...

Copy link
Member

@perlpunk perlpunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks good (after I read about what slots are)!

@perlpunk perlpunk added this to Backlog in 5.3 Release Dec 6, 2019
@perlpunk
Copy link
Member

perlpunk commented Dec 7, 2019

I added #161 to release/5.3

@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/

@perlpunk perlpunk closed this Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants