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

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

Closed
llucax opened this issue Jul 1, 2024 · 5 comments
Closed

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

llucax opened this issue Jul 1, 2024 · 5 comments
Labels
resolution:wontfix This will not be worked on type:enhancement New feature or enhancement visitble to users
Milestone

Comments

@llucax
Copy link
Contributor

llucax commented Jul 1, 2024

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

Originally posted by @llucax in #302 (review)

@llucax llucax added the type:enhancement New feature or enhancement visitble to users label Jul 1, 2024
@llucax llucax added this to the v1.1.0 milestone Jul 1, 2024
@shsms
Copy link
Contributor

shsms commented Jul 1, 2024

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
Copy link
Contributor Author

llucax commented Jul 1, 2024

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
Copy link
Contributor Author

llucax commented Jul 1, 2024

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

@shsms
Copy link
Contributor

shsms commented Jul 2, 2024

(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.

@shsms
Copy link
Contributor

shsms commented Jul 9, 2024

We decided not to do this, so that we don't have properties that can raise.

@shsms shsms closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
@llucax llucax modified the milestones: v1.1.0, Dropped Jul 9, 2024
@llucax llucax added the resolution:wontfix This will not be worked on label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:wontfix This will not be worked on type:enhancement New feature or enhancement visitble to users
Projects
None yet
Development

No branches or pull requests

2 participants