Add contains method to BytesType #126
Conversation
|
See #128 for a veiled feature request. |
|
@slott56 thanks for approving! gentle ping to get this merged since I cannot :) |
src/celpy/celtypes.py
Outdated
| return f"{self.__class__.__name__}({super().__repr__()})" | ||
|
|
||
| def contains(self, item: Value) -> BoolType: | ||
| return BoolType(cast(StringType, item) in self) |
There was a problem hiding this comment.
This isn't passing tests (for me, or in CI) Perhaps we should do this instead?
| return BoolType(cast(StringType, item) in self) | |
| return BoolType(BytesType(item) in self) |
There was a problem hiding this comment.
Or maybe you meant for this to be cast(BytesType, item) but if so you'd want to do assert b_0.contains(b"byte") below...
There was a problem hiding this comment.
Okay, did a bit more testing, and it seems like this is what's necessary to appease the type checker (and what's consistent with other examples of contains()):
| return BoolType(cast(StringType, item) in self) | |
| return BoolType(cast(bytes, item) in cast(bytes, self)) |
There was a problem hiding this comment.
hm, I'm mostly basing this on the StringType impl; why do we need to cast to bytes instead?
There was a problem hiding this comment.
@stefanvanburen logically, a string is a sequence of Unicode code points and there is always an encoding choice to be made to represent it as bytes.
3 participants
Noticed this method was missing while attempting to upgrade protovalidate-python to v0.4: https://github.com/bufbuild/protovalidate-python/actions/runs/16524843617/job/46735392706?pr=340#step:6:136.
Still blocked on a couple things (#113, google-re2 adding a Python 3.13 wheel), but figured I'd get this added in the meantime.