Refactor DBcore without changing behaviour#959
Conversation
Previously `normalize()` was used to convert values set by the model API consumer and values received from the database. These are two orthogonal uses. The commit does not change behaviour since the `from_sql()` method uses the `normalize()` implementation.
Instead of encoding the conversion behaviour in the model class (via the `_bytes_keys` attribute) we define it on the type. This gives us a more extensible interface and separates logic. This should not change any behaviour (as one can see by closely staring at the code).
We want to move common code into the base class with out changing the behaviour of the default type.
Instead of implementing the `parse()` and `format()` methods and setting the `null` attribute in subclasses, we set the `model_type` field and the generic implementations of the two methods take care of the rest. Again: No change in behaviour!
There was a problem hiding this comment.
Small typo: looks like there are now two if value is None: cases.
There was a problem hiding this comment.
Just after posting this, I realized why you did this. My mistake—sorry for the noise!
|
This is all absolutely stellar stuff. ✨ I like all the changes; these do have a large impact on readability. And extra points for the nice organization into atomic commits. Woohoo! I asked one question above, which is more relevant to the next stage in this process than this refactoring. I'd like to merge this now (I'm still working on #953 and it seems like a recipe for conflicts) and I'll make that one tiny Thanks again! |
Refactor DBcore without changing behaviour
Yes, I think so. Since the user might change the type of a flexible attribute at will we should at least make the database schema consistent. Then we only need to deserialize strings. For now I updated the documentation to reflect the current state. |
|
OK, sounds like a plan. |
2 participants
This PR contains some of the refactoring from #919. It extends the
TypeAPI and clearly separates responsibilities. The new API allows us to remove a lot of similar code.For a review I suggest to first look at the final
types.pyfile and the changes in the model implementation, modulo the changes in 19d3ca8. This will get you an overview of the goal. Then each commit should be reviewed. The changes are atomic and should leave the behaviour of the program unchanged.