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

Process serialization will fail if it contains an instance of ExitCode #3939

Closed
sphuber opened this issue Apr 17, 2020 · 2 comments · Fixed by #3940
Closed

Process serialization will fail if it contains an instance of ExitCode #3939

sphuber opened this issue Apr 17, 2020 · 2 comments · Fixed by #3940

Comments

@sphuber
Copy link
Contributor

sphuber commented Apr 17, 2020

This is because the ExitCode was changed from simple tuple to a class in v1.2.0 which now causes yaml.dump to except

import yaml
from aiida.engine import ExitCode
exit_code = ExitCode()
yaml.dump(exit_code)

will raise

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-33b699827b3a> in <module>
----> 1 yaml.dump(exit_code)

~/.virtualenvs/aiida_dev/lib/python3.7/site-packages/yaml/__init__.py in dump(data, stream, Dumper, **kwds)
    288     If stream is None, return the produced string instead.
    289     """
--> 290     return dump_all([data], stream, Dumper=Dumper, **kwds)
    291 
    292 def safe_dump_all(documents, stream=None, **kwds):

~/.virtualenvs/aiida_dev/lib/python3.7/site-packages/yaml/__init__.py in dump_all(documents, stream, Dumper, default_style, default_flow_style, canonical, indent, width, allow_unicode, line_break, encoding, explicit_start, explicit_end, version, tags, sort_keys)
    276         dumper.open()
    277         for data in documents:
--> 278             dumper.represent(data)
    279         dumper.close()
    280     finally:

~/.virtualenvs/aiida_dev/lib/python3.7/site-packages/yaml/representer.py in represent(self, data)
     25 
     26     def represent(self, data):
---> 27         node = self.represent_data(data)
     28         self.serialize(node)
     29         self.represented_objects = {}

~/.virtualenvs/aiida_dev/lib/python3.7/site-packages/yaml/representer.py in represent_data(self, data)
     32 
     33     def represent_data(self, data):
---> 34         if self.ignore_aliases(data):
     35             self.alias_key = None
     36         else:

~/.virtualenvs/aiida_dev/lib/python3.7/site-packages/yaml/representer.py in ignore_aliases(self, data)
    137         if data is None:
    138             return True
--> 139         if isinstance(data, tuple) and data == ():
    140             return True
    141         if isinstance(data, (str, bytes, bool, int, float)):

~/code/aiida/env/dev/aiida-core/aiida/engine/processes/exit_code.py in __eq__(self, other)
     50 
     51     def __eq__(self, other):
---> 52         return all(getattr(self, attr) == getattr(other, attr) for attr in ['status', 'message', 'invalidates_cache'])
     53 
     54 

~/code/aiida/env/dev/aiida-core/aiida/engine/processes/exit_code.py in <genexpr>(.0)
     50 
     51     def __eq__(self, other):
---> 52         return all(getattr(self, attr) == getattr(other, attr) for attr in ['status', 'message', 'invalidates_cache'])
     53 
     54 

AttributeError: 'tuple' object has no attribute 'status'

@greschd
Copy link
Member

greschd commented Apr 17, 2020

I think your traceback is missing the last part:

AttributeError: 'tuple' object has no attribute 'status'

It seems the ExitCode is somehow converted to (and then compared to) a bare tuple, which doesn't have the status attribute.

@greschd
Copy link
Member

greschd commented Apr 17, 2020

Changing ExitCode.__eq__ to include the type seems to work:

def __eq__(self, other):
    return type(self) == type(other) and all(
        getattr(self, attr) == getattr(other, attr) for attr in ['status', 'message', 'invalidates_cache']
    )

EDIT: It seems namedtuple compares equal to a bare tuple as long as the content is the same - maybe we should do the same. Easiest way to achieve that would just be remove our custom __eq__ implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants