Add type checking to strings/bytes for Python/Ruby #1597
Conversation
Also fixes that bytes had to be valid UTF-8 due to mozilla#1595.
|
Test failure is the unrelated #1553. |
| @@ -8,5 +8,9 @@ def read(buf): | |||
|
|
|||
| @staticmethod | |||
| def write(value, buf): | |||
| try: | |||
There was a problem hiding this comment.
Why not isinstance() here? I assume it's because you want to support anything exposing the buffer protocol - so why do bytes get "anything bytes-like" but strings aren't "anything string-like"?
There was a problem hiding this comment.
assume it's because you want to support anything exposing the buffer protocol
Yes, basically that.
I assume it's because you want to support anything exposing the buffer protocol - so why do bytes get "anything bytes-like" but strings aren't "anything string-like"?
Because I'm trying to mirror Python's native semantics here. Take e.g. the lambda x: str(x, encoding="utf8") function. It allows x to be anything adhering to the buffer protocol. But lambda x: bytes(x, encoding="utf8") only allows x to be str, no other classes pretending to be str. This also seems to be the case for other functions in Python that take bytes or str: Functions taking bytes usually take anything implementing the buffer protocol, functions taking str only allow str.
| self.assertRaises(TypeError, lambda: take_string(0.0)) | ||
| self.assertRaises(TypeError, lambda: take_string(b"")) | ||
|
|
||
| class A: |
There was a problem hiding this comment.
It's not clear to me what the current semantics are - does this work without this patch? If so, what's the motivation for preventing this from working?
There was a problem hiding this comment.
(I mean, I guess "does the object allow str(ob)?" isn't really a great indicator of "this object wants to be seen as string like", but my question is really more about "do we want to exclude objects which want to be string like?"
There was a problem hiding this comment.
It's not clear to me what the current semantics are - does this work without this patch?
I didn't change the current semantics except for throwing TypeError instead of AttributeError. So no, it does not work without this patch.
In this way, this test is trying to test for the current behavior.
You haven't missed a discussion on this. I probably should have expanded on the choices I made in the commit message. |
|
Thanks for the explanation. I'm fine with this, but I think you should wait for @badboy to say he is :) |
Also fixes that bytes had to be valid UTF-8 due to #1595.