-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add serialization upgrade script for type renaming #3303
Add serialization upgrade script for type renaming #3303
Conversation
7fe3265
to
a103b6c
Compare
Ok, so this works now locally with some examples that I serialized on 56feead (i.e. before #3231) and loaded them on this branch. @antonydellavecchia can you please have a look if what I've done here looks more or less sensible? You don't have to check the concrete types to update, just the general framework of renaming types. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things look more or less correct. If tests are passing then great. I was a bit concerned about the fact you need to update s.refs after the upgrade, but this is probably something for me to look into. Thanks for doing this!
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3303 +/- ##
===========================================
+ Coverage 41.65% 81.60% +39.94%
===========================================
Files 533 547 +14
Lines 71640 73530 +1890
===========================================
+ Hits 29844 60005 +30161
+ Misses 41796 13525 -28271
|
* Revert JohnsonMatrices to an older state * Reload refs after upgrade script * Add serialization upgrade script for type renaming
This approaches to add an upgrade script for serialized files incorporating the renamings from #3231 and #3288. A script for applying the renaming can is available in #3288 (comment).
I looked through all occurrences of
@register_serialization_type
in the codebase and compared them with the renaming script. If the type got renamed, both old and new names got added to the list in the serialization code.Everything got a bit finicky, as type strings can be at a lot of different places, and furthermore inside
:_refs
and this needs to get reloaded after the upgrading into the DeserializationState.I reverted to
data/JohnsonMatrices/
folder to before #3288. They should get either kept at this older state or upgraded once this PR works usingcc @thofma @joschmitt @antonydellavecchia