Skip to content

Commit

Permalink
add to_jdict and from_jdict to Line
Browse files Browse the repository at this point in the history
  • Loading branch information
Valentin Féron committed Apr 4, 2023
1 parent d69c251 commit 5d88378
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions lined/base.py
View file
Expand Up @@ -609,6 +609,19 @@ def __repr__(self):
def _name_of_instance(self):
return getattr(self, "__name__", self.__class__.__name__)

def to_jdict(self):
return {
'funcs': self.funcs,
'pipeline_name': self.name,
'input_name': self.input_name,
'output_name': self.output_name
}

@classmethod
def from_jdict(cls, jdict):
funcs = jdict.pop('funcs')
return cls(*funcs, **jdict)


Pipeline = Line # for back-compatibility

Expand Down

2 comments on commit 5d88378

@thorwhalen
Copy link
Member

Choose a reason for hiding this comment

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

The to_jdict and from_jdict thing.

I know, I standard that trend in CentroidSmoothing.

It has its advantages (we can just ask the object itself to give us a jsonizable dict, or make itself with one).

But it also has some draw backs.

For one, it’s not separating the classes’ main purpose(s) and the json-serialization concern.

There are other options.

Still not separating the serialization concern, but doing it the python way, would be to use

__setstate__
__getstate__

instead. But note that then pickle would use that too, which is maybe not what we want.

One of the purposes of py2json is to make it easy(er) to externalize the json-serialization concern. We could, for example,

  • use the tools in the json lib to inform json.load and json.dump about the codec to use at the point we need to.
  • decorate a class or instance with the information it needs so that serialization can use it when and if needed

@valentin-feron
Copy link
Member

Choose a reason for hiding this comment

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

Yes I know it is not ideal.

We could put this logic into a decorator to add those methods. It is not ideal either becuase, sometimes, you just want to serialize an object without having to decorate it first to make sure it is serializable.

Regarding the __setstate__/__getstate__ solution, yes, it sounds dangerous to change the way pickle will serialize it. But it also sounds consistent to have only one way to serialize a class.

Please sign in to comment.