Refactor: don't pass attr, obj, data to fields (de)serialize methods#2963
Draft
Refactor: don't pass attr, obj, data to fields (de)serialize methods#2963
Conversation
Member
Author
|
I don't remember the rationale for not having mypy run in pre-commit. |
Member
|
only skimmed the code changes (haven't read the tests), but this is looking really nice. i do think this is the right direction overall. maybe rename to
👍 that seems like a good release plan |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2039 is a long standing issue I've been wanting to address for a while but it implies a lot of changes that can hardly be done incrementally so I never got down to it.
[Disclaimer: The company I work for wants us to leverage AI power because everyone does it (FOMO) so we asked for Claude tokens and I tried agent coding on our projects to give management feedback about it and I figured why not try it on this issue as well. Obviously, it is not as simple as telling the tool to fix #2039. It took me a few tries to get the steps in the right order. And although I was supervising, there may be a few hallucinations I haven't spotted yet. The tool was not always clever at guessing what I didn't tell, but it helped me facing the fear of rewriting all calls to serialization functions in lib and tests.]
I'm sharing this as it is a nice POC. It still needs to be polished around the edges and there may be possibilities for further improvements, but some of them could be done later. Changes certainly need to be double-checked (but this would also be true if I had done all this manually, TBH). Consider it an illustration of what I have in mind.
The [de|]serialization_getter arguments added to
Fieldcould be added in MA4 while deprecating the use ofFunction/Methodto do fancy accessing. The rest is breaking changes that would have to wait until 5.0.Happy to get feedback before doing any more work on this.