Skip to content
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

Refactor Field.serialize() #799

Closed
deckar01 opened this issue May 8, 2018 · 5 comments
Closed

Refactor Field.serialize() #799

deckar01 opened this issue May 8, 2018 · 5 comments

Comments

@deckar01
Copy link
Member

deckar01 commented May 8, 2018

#788 was caused by confusion between Field.serialize() and Field._serialize().

Field.serialize() plucks a value out of a schema level object and passes it on to Field._serialize(). Field.serialize() is only used in Marshaller.serialize().

I propose moving Field.serialize() to Marshaller.serialize_field() and Field.deserialize() to Marshaller.deserialize_field() to avoid this confusion.

@sloria
Copy link
Member

sloria commented May 14, 2018

Seems like a sensible change. I'll mark it for 3.0, since it would be a backwards-incompatible change, though I'm not sure this needs to be a hard blocker for 3.0 final.

@lafrech
Copy link
Member

lafrech commented Sep 1, 2018

@deckar01 we may postpone this, unless you think it is an easy job.

@sloria
Copy link
Member

sloria commented Dec 30, 2024

I still think this would be a good change. I don't think it's a must-have for 4.0, but would be nice. @deckar01 would you be interested in implementing this?

@sloria
Copy link
Member

sloria commented Jan 3, 2025

this is part of what you're proposing in #1046, right? should we close this one and continue discussion in that issue?

@lafrech
Copy link
Member

lafrech commented Jan 3, 2025

It is related, indeed. Could be a first step. Or we may want to deal with the whole thing in one go. No problem with closing this in favor of #1046.

@sloria sloria closed this as completed Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants