codecov[bot]
commented
1 year ago Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
f87e236) 99.18% compared to head (42565aa) 99.20%.
Additional details and impacted files
```diff @@ Coverage Diff @@ ## main #227 +/- ## ========================================== + Coverage 99.18% 99.20% +0.02% ========================================== Files 36 36 Lines 1843 1892 +49 ========================================== + Hits 1828 1877 +49 Misses 15 15 ```:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
dwhswenson
richardjgowers
pep8speaks
GufeTokenizableTestMixinto tests of migration processesWe decided that
GufeTokenizables need to be versioned, and that we need to provide a migration path to load old serialized representations of theGufeTokenizable. This PR provides the core functionality to do that.Changes to
GufeTokenizable_version: Should return a sortable marking the serialization version.serialization_migration: Takes a dict (possibly from another version of serialization, but otherwise suitable to be ingested byGufeTokenizable._from_dict) and a version, and converts the dict to ensure that it can be handled by the current version of the code.Changes to the serialized representation of
GufeTokenizablesNow serialized versions of
GufeTokenizables (described as a Python dict/JSON object) will carry the additional key":version:", which describes the version. Serialized representations without that key will be treated as if its value was1. Like__module__and__qualname__, this is added/popped without intervention from contributors.Labelling versions
I'm proposing we use versions that are simple increasing integers. In principle, users could use anything. However, I think that semantic versioning has no meaning here (the only purpose would be to distinguish breaks that can no longer be handled by the code, which is probably as easily handled with a simple
version < oldest_supported_version). Tying to the version of the package also makes it more tedious to handle updates: first, this means that it requires more careful attention to the version each new thing was added in. Second, this is a nightmare for dev installs, because they lie about version number.On reorganizing keys within nested structures
Although we identified this as target, I don't think this will be easily done with general
GufeTokenizables. The issue is that, in deserialization, we can use gufe keys to shortcut through the tree of nested object, gaining some performance. However, if we want to rearrange the tree of the nested structure, we need to reconstruct the entire structure, and the make the modifications. This is a significantly different process than the one we use now, and may be slower.That said, moving an attribute within a digraph is probably an indication that the original design was significantly flawed, which is not something I think we need to worry about with
GufeTokenizables. For example, I have a hard time imagining that we would want to transferTransformation.protocol.settingstoTransformation.chemical_systems.protocol_settingsor transfer a hypotheticalAlchemicalNetworkCreator.mapper.timeouttoAlchemicalNetworkCreator.timeout.Where this will matter is in
Settings, where we expect rapid flux in the near future. However,Settingsare notGufeTokenizables. Therefore, their "key-encoded" representation is still a nested dict, and the parentGufeTokenizable(theProtocol) can manage migrations. As of opening this PR, tools to facilitate that haven't been added; that will be a future step, if this PR isn't nixed.With this PR, changes in settings will require changes in all parent protocols. We could additionally create similar versioning of
Settings, which would duck-type the same as the approach here, but build on tools within the JSON codecs instead of the coreGufeTokenizable. This can be done, but would be part of a separate PR. That said, a change in organization (moving an attribute from cousin to cousin) will always require that the correction be handled at a common ancestor (grandparent). Purely local changes are not possible for a change that is fundamentally nonlocal.Sequential migrations
This isn't illustrated here, but the pattern to use is actually something like this:
This does a series of sequential migrations, and continues to support version 1, 2, and 3 with minimal code. If we find that the chain of versions gets too long, it might be worth creating some skips in there, but I kind of doubt it will ever be worth going to the "correct" solution of creating the directed network and finding the minimal path.