Skip to content

Conversation

@olijeffers0n
Copy link
Contributor

@olijeffers0n olijeffers0n commented Apr 25, 2023

Closes #479 by attempting to set the serialization state upon setting the attribute. I am not familiar with this codebase however I believe this should work properly, and does from my testing.

For context, the previous approach only set the serialisation state to be truthy when an attribute of this "child" message was updated. This does not work for empty message structures so it needs to be set when the new object is assigned instead.

@Gobot1234
Copy link
Collaborator

I think this is better handled in __post_init__ no?

@olijeffers0n
Copy link
Contributor Author

I thought about that, but i’m not sure how that would work as I do not want the instance that is assigned statically to be serialised. If you know how to fix this better, please supersede this PR

@olijeffers0n
Copy link
Contributor Author

@Gobot1234 How would you like to proceed with this / what can I do better here?

@olijeffers0n
Copy link
Contributor Author

Looks good to me

@Gobot1234
Copy link
Collaborator

Would you mind adding a test case for this regression? (and also fixing the formatting better than I did 😉 poe fmt)

@olijeffers0n
Copy link
Contributor Author

Thanks for reviewing - I am in the middle of exams at the moment though so won’t be able to get to this for a while. Plus, I don’t really know how to write tests but i could work that out

@Gobot1234
Copy link
Collaborator

@olijeffers0n friendly poke, if you don't have time to finish this I can take over if you want

@olijeffers0n
Copy link
Contributor Author

Yeah, I have time I just don’t know how to write unit tests. I’m happy for you to take over

Gobot1234
Gobot1234 previously approved these changes Jun 24, 2023
@Gobot1234
Copy link
Collaborator

Thanks

@Gobot1234 Gobot1234 merged commit 182aeda into danielgtaylor:master Jun 24, 2023
bbonenfant pushed a commit to pachyderm/python-betterproto that referenced this pull request Jan 23, 2024
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
bbonenfant pushed a commit to pachyderm/python-betterproto that referenced this pull request Jan 23, 2024
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some message structures are not serialised properly

2 participants