search

frequenz-floss / frequenz-channels-python

Channel implementations for Python
https://frequenz-floss.github.io/frequenz-channels-python/
MIT License
7 stars 8 forks source link

Rename `LatestValueCache.get()` to `value` and make it a `@property` #306

Open llucax opened 5 days ago

llucax commented 5 days ago

I think I could make sense to rename get() to value and make it a @property.

Originally posted by @llucax in https://github.com/frequenz-floss/frequenz-channels-python/pull/302#pullrequestreview-2151374660

shsms commented 5 days ago

I guess the only concern is that this could raise an exception when no value has been received. It feels a bit weird to have properties that can raise.

llucax commented 5 days ago

Interesting point. I'm pretty sure we have some properties that raise exceptions, I always thought it was fine because accessing an attribute ultimately can always raise a AttributeError, but when using type hints and a checker, that really shouldn't happen.

On the other hand, I like the simplicity of a property syntax when the expected case is that it should work (this should only raise during "initialization"), which I think is kind of the pattern I've been using.

Probably this needs some discussion. Ideally I would like to make a decision and try to stick with it so we have some consistency about this.

llucax commented 5 days ago

@frequenz-floss/python-sdk-team FYI.

shsms commented 4 days ago

(this should only raise during "initialization")

but that could be a long time, and sometimes not even when expected, because of broken data source or connection.