Fix several issues #12
Conversation
|
/cc @jckenny59 |
| @@ -103,6 +113,26 @@ def __init__(self, name, message_type, **options): | |||
| self.message_type = message_type | |||
| self.options = options | |||
|
|
|||
| def _message_to_json(self, message): | |||
| """Convert a message to a JSON string. | |||
There was a problem hiding this comment.
should this (and _message_from_json) maybe made public? I see they are only used externally.
more generally, aren't these two doing something very similar to compas.data?
There was a problem hiding this comment.
they are doing something similar to compas.data but not exactly, a message can be a simple dictionary, an instance of Message, or a subclass of compas.data.Data and needs to support compas 1.x and compas 2.x
src/compas_eve/core.py
Outdated
| except (KeyError, AttributeError): | ||
| try: | ||
| data = dict(message) | ||
| except Exception: |
There was a problem hiding this comment.
maybe catch more specific errors like ValueError and TypeError?
There was a problem hiding this comment.
true, I was too lazy to think which one is the correct exception here :P
Thanks! I think I fixed all the review findings. About the (de)serialization of |
Solves #11
This PR fixes several things: first of all, the base case of the
Messageclass was removed and re-based toobjectbecauseUserDictis an old-style class and that creates a ton of problems.What type of change is this?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas.datastructures.Mesh.