Complete serialization overhaul #263
Conversation
|
Maybe introduce a new method |
…oth transition. New classes are now called JSONSerializableDecoder/-Encoder .
There was a problem hiding this comment.
In general most of the changes make a lot of sense, especially moving code from the old Serializer to JSONDecoder/-Encoder extensions. I would, however, rename the ExtendedJSONDecoder/-Encoder classes to a more specific name (e.g. JSONSerializableDecoder/-Encoder) but that's a minor thing (but would allow us to have old and new routines in co-existence for a transition period).
The only thing I really don't like here is the central pulse register. It makes object construction more inconvenient and isn't even guaranteed to detect duplicate ids: if a pulse with some id exists in the storage but is never loaded in an application, it will not appear in the register... we need to check for that in the PulseStorage or StorageBackend anyways, so there is no benefit to the register.
|
I also tried to think about a transition period where the old routines are still available but deprecated but I don't really want to rename the relevant methods in the new implementations (and can't in the old one) (to_dict is much less precise a name then get_serialization_data() )... it would be nice if Python would support method overloading but meh |
…ible with old for transition period and marked old routines as deprecated.
|
Branch issues/250_serialization_backward_compatability contains my current state of implementing the corresponding methods in PTs for the new ecosystem but for now backward compatible (and deprecated). |
All now implement get_serialization_data and deserialize for old and new serialization routines for a transition period.
Methods did not call Serializable::get_serialization_data() and thus did not inculde #type and #identifier keys in the returned dictionaries. Tests did not cover this. Moved identifier_name and type_identifier_name from JSONSerializableEncoder/-Decoder to Serializable. Signed-off-by: Lukas Prediger <lukas.prediger@rwth-aachen.de>
Implementation was inconsistent with _temporary_storage usage in other places.
DummySerializable now behaves as follows: Takes any number of keyword-arguments and stores them as class attributes. get_serialization_data() returns a dictionary with (key, value) pairs for all attributes given this way. This is compatible with previous usage (where only a "data" attribute of type Any was given and get_serialization_data() returned a dictionary with only key "data" and whatever given value). Removed duplicated definition of DummySerializable further down the same file. NestedDummySerializable now is also compatible with new serialization routines and implements deserialize().
..to adapt syntax/type to behavior. Slight change in DummySerializableTests.make_kwargs to extend testing of DummySerializable funcitonality a bit.
…format. Pulse is read using old serialization routines from a source storage backend and destination storage backend to a new using the new routines.
Method should list all available serializables. No tests currently.
convert_pulses_in_storage converts all pulses in a given source StorageBackend from the old serialization format into the new and stores them in a given destination StorageBackend. Both conversion methods now raise an exception if they would need to overwrite identifiers already existing in the destination backend. Commit also includes more extensive tests for both conversion methods as well as some style corretions for already existing tests.
…global registry after they are completely initialized. Serializable now does not perform registration in __init__ but provides a method _register() which MUST be called by leaf-level subclasses. (This is probably also not a good solution in the long term as it effectively prevents further subclassing from those classes but it works for now). Signed-off-by: Lukas Prediger <lukas.prediger@rwth-aachen.de>
|
We should be about to wrap this up. Can merge into master after incorporating pull requests |
Implementations of __len__ and __iter__ curently raise NotImplementedError s. Signed-off-by: Lukas Prediger <lukas.prediger@rwth-aachen.de>
…_conversion_script
Issues/299 default registry none
…on_script Serialization conversion script
…id_inconsistencies
…istencies Issues/272 pulse storage id inconsistencies
Issues/298 early pulse registration
…pe_annotations_for_registry
…egistry occurrences accordingly.
…tions_for_registry Issues/308 consistent type annotations for registry
…nt over several instantiations.
Increased test coverage for serialization
terrorfisch
changed the title
|
let's go |
The aim of this overhaul is to make Serialization more future proof and lay the groundwork for easier pulse management. Also solves #250 completely.
dictinterface where ever it makes sense (StorageBackends i.e.)json.JSONEncoderandjson.JSONDecoder. This leads to much less code in eachPulseTemplatebut requries that references are marked explicitly.Some of these are backward incompatible and need to be discussed in detail.