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

Don't allow checking if None is within the bounds #1140

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Jan 2, 2025

This was never supported, a ValueError would be raised if None was passed to __contains__. Now we assert to make it clear that it's not allowed.

In the future we should probably change Bounds to always allow None as bounds and make _T simply bound to Comparable, as supporting None depending on the generic type makes it very hard to provide correct type hints.

This was never supported, a `ValueError` would be raised if `None` was
passed to `__contains__`. Now we assert to make it clear that it's not
allowed.

In the future we should probably change `Bounds` to always allow `None`
as bounds and make `_T` simply bound to `Comparable`, as supporting
`None` depending on the generic type makes it very hard to provide
correct type hints.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner January 2, 2025 14:17
@llucax llucax requested review from shsms and removed request for a team January 2, 2025 14:17
@github-actions github-actions bot added the part:data-pipeline Affects the data pipeline label Jan 2, 2025
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jan 2, 2025
@llucax llucax self-assigned this Jan 2, 2025
@llucax llucax enabled auto-merge January 2, 2025 14:18
@llucax llucax added this to the v1.0.0-rc1500 milestone Jan 2, 2025
We only need to do this when the cache directory exists, otherwise the
step fails with an error.

Signed-off-by: Leandro Lucarella <[email protected]>
@github-actions github-actions bot added the part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) label Jan 2, 2025
@llucax
Copy link
Contributor Author

llucax commented Jan 3, 2025

This doesn't change behavior, we can decide if we want to change it in a separate PR, but I will merge this for now to unblock the merging of the mypy update.

@llucax llucax disabled auto-merge January 3, 2025 08:51
@llucax llucax merged commit fcb7de4 into frequenz-floss:v1.x.x Jan 3, 2025
18 checks passed
@llucax llucax deleted the bounds-contains branch January 3, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd:skip-release-notes It is not necessary to update release notes for this PR part:data-pipeline Affects the data pipeline part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)
Projects
Development

Successfully merging this pull request may close these issues.

1 participant