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

value_receiver vs _value_receiver? #566

Closed
XzzX opened this issue Jan 20, 2025 · 4 comments · Fixed by #569
Closed

value_receiver vs _value_receiver? #566

XzzX opened this issue Jan 20, 2025 · 4 comments · Fixed by #569
Labels
bug Something isn't working

Comments

@XzzX
Copy link
Contributor

XzzX commented Jan 20, 2025

class DataChannel(FlavorChannel["DataChannel"], typing.Generic[ReceiverType], ABC):
    def __init__(
        self,
        label: str,
        owner: HasIO,
        default: typing.Any | None = NOT_DATA,
        type_hint: typing.Any | None = None,
        strict_hints: bool = True,
        value_receiver: ReceiverType | None = None,
    ):
        ...
        self.value_receiver: ReceiverType = value_receiver

    @property
    def value_receiver(self) -> Self | None:
        return self._value_receiver

Is this a bug, or is this intended? value_receiver vs _value_receiver?

@XzzX XzzX added the bug Something isn't working label Jan 20, 2025
@liamhuber
Copy link
Member

liamhuber commented Jan 20, 2025

This is completely intended. value_receiver is a setter/getter property. The underlying _value_receiver is earlier set to some default value, and this pattern is leveraged to have the setter sanitize whatever value_receiver arg the user has passed into __init__

    def __init__(
        ...
        value_receiver: ReceiverType | None = None,
    ):
        ...
        self._value_receiver = None
        ....
        self.value_receiver: ReceiverType = value_receiver

    ...

    @value_receiver.setter
    def value_receiver(self, new_partner: Self | None):
        if new_partner is not None:
            ...

@XzzX
Copy link
Contributor Author

XzzX commented Jan 21, 2025

self.value_receiver: ReceiverType = value_receiver

The type hint is misleading as it suggests a declaration shadowing your property.
You could also get rid of ReceiverType entirely as you only allow Self.

@liamhuber
Copy link
Member

You're right that the attribute doesn't need a hint since the property has one, but the property hint should actually be ReceiverType not Self. We can't get rid of ReceiverType because there is a subclass of the output data channel: #569

@XzzX
Copy link
Contributor Author

XzzX commented Jan 22, 2025

You're right that the attribute doesn't need a hint since the property has one, but the property hint should actually be ReceiverType not Self. We can't get rid of ReceiverType because there is a subclass of the output data channel: #569

Fallen into my own trap... I am still confused by the meaning of Channel. But yes, of course, you are right.

@liamhuber liamhuber linked a pull request Jan 22, 2025 that will close this issue
@XzzX XzzX closed this as completed in #569 Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants