ezmsg-org / ezmsg

Pure-Python DAG-based high-performance SHM-backed pub-sub and multi-processing pattern
https://ezmsg.readthedocs.io/en/latest/
MIT License
9 stars 4 forks source link

ez.Flag should be a dataclass, not deprecated Message. #95

Closed cboulay closed 4 months ago

cboulay commented 4 months ago

I ran into this when trying to log-and-read ez.Flag messages.

import json
import ezmsg.core as ez
from ezmsg.util.messagecodec import MessageEncoder, MessageDecoder

if __name__ == "__main__":
    strmessage: str = json.dumps(ez.Flag, cls=MessageEncoder)
    obj = json.loads(strmessage, cls=MessageDecoder)  # Raise TypeError

TypeError: MessageMeta.__new__() missing 3 required positional arguments: 'name', 'bases', and 'classdict'

This is because the only 2 fields in obj are TYPE and TIMESTAMP_ATTR, so the final obj in out_obj = cls(**obj) is an empty dict without the required arguments to instantiate a MessageMeta obj. However, I highly doubt we want to instantiate a MessageMeta; we'd probably want to instantiate a Message but this is deprecated anyway.

cboulay commented 4 months ago

I think I was just mislead. In the following line,

https://github.com/iscoe/ezmsg/blob/db6b3b3bf87c9ffe321c516b06a75961c95131af/extensions/ezmsg-sigproc/src/ezmsg/sigproc/synth.py#L47

Should it have been ez.Flag() instead?

cboulay commented 4 months ago

Actually, changing the problem-reproducing code to use ez.Flag() raises a different error.

import json
import ezmsg.core as ez
from ezmsg.util.messagecodec import MessageEncoder, MessageDecoder

if __name__ == "__main__":
    strmessage: str = json.dumps(ez.Flag(), cls=MessageEncoder)
    obj = json.loads(strmessage, cls=MessageDecoder)  # Raise TypeError
File "ezmsg/src/ezmsg/util/messagecodec.py", line 118, in object_hook
    setattr(out_obj, TIMESTAMP_ATTR, timestamp)
  File "<string>", line 4, in __setattr__
dataclasses.FrozenInstanceError: cannot assign to field '_log_timestamp'

I replaced the definition of Flag with

@dataclass
class Flag:
    """Message with no contents"""
    ...

And this seems to work fine.

griffinmilsap commented 4 months ago

Thanks for addressing this; you fixed it in the now merged PR #53