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

bytes should implement __new__ #2630

Closed
mystor opened this issue Nov 25, 2018 · 6 comments
Closed

bytes should implement __new__ #2630

mystor opened this issue Nov 25, 2018 · 6 comments
Labels
stubs: incomplete Annotations or sub-modules missing from an existing package or module

Comments

@mystor
Copy link

mystor commented Nov 25, 2018

This seems like a similar issue to #2091. The bytes type implements __init__ but doesn't appear to provide an overload for __new__. This makes it harder for subclasses of bytes to define their own behaviour for __new__.

class bytes(ByteString):
@overload
def __init__(self, ints: Iterable[int]) -> None: ...
@overload
def __init__(self, string: str, encoding: str,
errors: str = ...) -> None: ...
@overload
def __init__(self, length: int) -> None: ...
@overload
def __init__(self) -> None: ...
@overload
def __init__(self, o: SupportsBytes) -> None: ...

This also appears to be the case for some of the other builtins (e.g. str)

class str(Sequence[str]):
@overload
def __init__(self, o: object = ...) -> None: ...
@overload
def __init__(self, o: bytes, encoding: str = ..., errors: str = ...) -> None: ...

@srittau
Copy link
Collaborator

srittau commented Nov 29, 2018

Could you give an example where this is a problem? bytes and str should inherit __new__ from object if I am not mistaken.

@mystor
Copy link
Author

mystor commented Nov 29, 2018

They do, but the argument list is wrong. As a strawman example:

class SHA1Hash(bytes):
    def __new__(cls, b: bytes) -> 'SHA1Hash':
        if len(b) != 20:
            raise ValueError("Invalid SHA1Hash value!")
        return super().__new__(cls, b)

This doesn't typecheck, because the call to super().__new__(cls, b) is inferred to be object.__new__(cls, b) which is incorrect as object.__new__ takes only the cls argument. bytes, on the other hand, takes an argument to create the object.

This is also a bit like the tuple case again in that __init__ doesn't make much sense for bytes or str, as they are non-mutable and can only be constructed with __new__ in the first place.

@srittau
Copy link
Collaborator

srittau commented Nov 29, 2018

Thanks for the explanation. Makes sense. We appreciate pull requests!

@srittau srittau added stubs: false positive Type checkers report false errors size-small labels Nov 29, 2018
@sproshev
Copy link
Contributor

str.__new__ had been added in #1352 but then reverted

@mystor
Copy link
Author

mystor commented Nov 30, 2018

Figure I should add a ref to the reason it was backed out: #1464

It appears it was backed out because the addition of __new__ to str caused problems for classes deriving from both str and Enum, as the str's __new__ would now take priority rather than the Enum's. I'm guessing this is caused by some magic wrt how str is actually implemented within python :-/

Perhaps there needs to be a special case for these builtins to make constructing them work correctly?

@srittau srittau added stubs: incomplete Annotations or sub-modules missing from an existing package or module and removed stubs: false positive Type checkers report false errors labels Sep 17, 2020
@hauntsaninja
Copy link
Collaborator

Closed by #4555 (and thank you for git-revise!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: incomplete Annotations or sub-modules missing from an existing package or module
Projects
None yet
Development

No branches or pull requests

4 participants