Skip to content

Selecting an Optional type during generation #73

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

Closed
yamatiro opened this issue Apr 25, 2025 · 2 comments
Closed

Selecting an Optional type during generation #73

yamatiro opened this issue Apr 25, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@yamatiro
Copy link

Summary

Perhaps it would be a better solution to add an argument for the compiler to choose whether to do so or not, because I have now switched from the first version of betterproto, and there the optional in the generated files is indicated only if the optional is specified in the proto files, and this breaks the transition

What is the feature request for?

The core library

The Problem

The Ideal Solution

The Current Solution

@yamatiro yamatiro added the enhancement New feature or request label Apr 25, 2025
@AdrienVannson
Copy link
Contributor

The problem is that it is not that easy: simply removing the optional type annotation in the compiler would not work.

If two messages A an B are defined, it is entirely possible that A has a (non-optional) field of type B, and B has a non-optional field of type A. Thus, the default value of these fields can't simply be an empty message.

The previous version of betterproto used to rely on a complex mechanism to approach this: the message fields were assigned a placeholder value. When the user was trying to access a field, the placeholder was dynamically replaced by an empty message (by redefining the __getattribute__ method of messages). To know is a message field was there because it was the default value or because it was actually part of the message, an artificial _serialized_on_wire field was added to each message.

This was the cause of many problems, some of them being impossible to solve with this design. Plus, the behavior was really unintuitive (simply reading a field could change a message).

You can see the PR that I did on the original repo for more details (it was not merged on this repo, only on the new one) : danielgtaylor/python-betterproto#641

I'm not sure if there is a simple workaround to mimic the behavior of the previous version without all the problems that came with it (maybe something like adding back a __getattribute__ function to messages to replace None by an empty message when the field is accessed could partially work as a workaround, I will have a look).

@yamatiro
Copy link
Author

@AdrienVannson Thanks for the explanation. Yes, now I understand the reason, your decision seems more correct and intuitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants