Skip to content

Conversation

@srittau
Copy link
Collaborator

@srittau srittau commented May 28, 2021

  • Extract _socket.pyi from socket.pyi.
  • Extract _socket.socket from socket.socket.
  • Fix socket.family annotation.
  • Annotate SocketIO properly.
  • SocketType is an alias of _socket.socket.
  • Sort items in socket.pyi in the same order as in socket.py.
  • Remove socket.EINTR.
  • Add socket.errorTab (Windows only) and _socket.dup().
  • Use _typeshed.WriteableBuffer instead of custom alias.
  • Use built-in generics.
  • Use socket.socket rather than socket.SocketType in stdlib and third-party stubs.

srittau added 9 commits May 28, 2021 13:34
* Sort items in socket.pyi in the same order as in socket.py.
* Remove socket.EINTR.
* Extract _socket.pyi from socket.pyi.
* Extract _socket.socket from socket.socket.
* Fix socket.family annotation.
* Annotate SocketIO properly.
* SocketType is an alias of _socket.socket.
* Sort items in socket.pyi in the same order as in socket.py.
* Remove socket.EINTR.
* Use _typeshed.WriteableBuffer instead of custom alias.
* Add errorTab (Windows only).
* Add _socket.dup().
* Use built-in generics.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@srittau
Copy link
Collaborator Author

srittau commented May 28, 2021

Looking at the primer failures:

  • anyio.abc._sockets wasn't flagged before, since it's the attributes dict contained an Any type before.
  • anyio._backends._asyncio wasn't flagged before, since UNIXSocketStream.__raw_socket was annotated as socket.SocketType, which used to be an alias for Any. The annotation is wrong and should be socket.socket, which should fix this.
  • paasta and ignite don't seem to have changes. I don't know why a diff is generated. Cc @hauntsaninja Do you know what's going on?
  • werkzeug: This points to a genuine problem that wasn't flagged before, since BaseRequestHandler.connection was SocketType, which was an alias for Any.

@JelleZijlstra
Copy link
Member

paasta and ignite don't seem to have changes. I don't know why a diff is generated. Cc @hauntsaninja Do you know what's going on?

I think what happens is that the order of the output changes. The order in which mypy outputs files sometimes changes for unclear reasons. Ideally the mypy-primer diff logic should detect when the errors just get reordered.

@srittau
Copy link
Collaborator Author

srittau commented May 28, 2021

I opened bpo-44261. SocketType is an alias for _socket.socket, when it is documented to be (and probably should be) an alias for socket.socket, a subclass.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, noticed some improvements in the C stubs

# Besides those and the first few constants listed, the constants are listed in
# documentation order.
import _socket
from _socket import (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just do from _socket import *? That's what happens at runtime, and it's easier to maintain than keeping that huge list of constants in sync.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this at first. See my frustrated comment about mypy not handling overrides of star imports correctly above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment to the import that explains this decision.


# enum versions of above flags
class AddressFamily(IntEnum):
AF_UNIX: int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This technically needs some more platform dependence. On Mac I only get these:

In [29]: socket.AddressFamily.__members__
Out[29]: 
mappingproxy({'AF_UNSPEC': <AddressFamily.AF_UNSPEC: 0>,
              'AF_UNIX': <AddressFamily.AF_UNIX: 1>,
              'AF_INET': <AddressFamily.AF_INET: 2>,
              'AF_SNA': <AddressFamily.AF_SNA: 11>,
              'AF_APPLETALK': <AddressFamily.AF_APPLETALK: 16>,
              'AF_ROUTE': <AddressFamily.AF_ROUTE: 17>,
              'AF_LINK': <AddressFamily.AF_LINK: 18>,
              'AF_IPX': <AddressFamily.AF_IPX: 23>,
              'AF_INET6': <AddressFamily.AF_INET6: 30>,
              'AF_SYSTEM': <AddressFamily.AF_SYSTEM: 32>})

I'm fine with leaving this as is though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspected that these are not up-to-date anymore. But I'm not keen on looking through this in detail at the moment.

srittau added 9 commits May 30, 2021 00:18
* Remove constructors.
* timeout is an alias for TimeoutError, starting with Python 3.10.
Use PEP 604 in changed lines
Use PEP 604 syntax for getaddrinfo()

Remove redundant bytearray annotation
This fixes stubtest warnings as those are positional-only in socket,
but not in SSLSocket.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

else:
_FD = SupportsInt

_CMSG = Tuple[int, int, bytes]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe in argument positions this can be any ReadableBuffer. This shows up in the anyio primer output; anyio is passing an array.

from collections.abc import Iterable
from typing import Any, Optional, SupportsInt, Tuple, Union, overload

if sys.version_info >= (3, 8):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit surprised by this but it seems right; PyLong_AsSocket_t is an alias for PyLong_AsLong, and that does __index__ on main but not on 3.7.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to claim that was intentional, but it was pure lazyness on my part, since I didn't want to write a custom protocol. Of course, I could have imported from typing_extensions ...

JelleZijlstra added a commit to JelleZijlstra/anyio that referenced this pull request May 30, 2021
SocketType is an alias for the private `_socket.socket` class, a superclass of `socket.socket`. It is better to just always use `socket.socket` in types. See https://bugs.python.org/issue44261 and python/typeshed#5545 for some context.
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

paasta (https://github.com/yelp/paasta.git)
+ paasta_tools/autoscaling/pause_service_autoscaler.py:5: error: Library stubs not installed for "pytz" (or incompatible with Python 3.7)
+ paasta_tools/autoscaling/pause_service_autoscaler.py:6: error: Library stubs not installed for "tzlocal" (or incompatible with Python 3.7)
+ paasta_tools/autoscaling/pause_service_autoscaler.py:6: note: Hint: "python3 -m pip install types-tzlocal"
- paasta_tools/autoscaling/pause_service_autoscaler.py:5: error: Library stubs not installed for "pytz" (or incompatible with Python 3.7)
- paasta_tools/autoscaling/pause_service_autoscaler.py:6: error: Library stubs not installed for "tzlocal" (or incompatible with Python 3.7)
- paasta_tools/autoscaling/pause_service_autoscaler.py:6: note: Hint: "python3 -m pip install types-tzlocal"

ignite (https://github.com/pytorch/ignite)
- ignite/contrib/handlers/visdom_logger.py:7: error: Cannot find implementation or library stub for module named "torch"  [import]
- ignite/contrib/handlers/visdom_logger.py:8: error: Cannot find implementation or library stub for module named "torch.nn"  [import]
- ignite/contrib/handlers/visdom_logger.py:9: error: Cannot find implementation or library stub for module named "torch.optim"  [import]
- ignite/contrib/handlers/visdom_logger.py:152: error: Cannot find implementation or library stub for module named "visdom"  [import]
- ignite/contrib/handlers/visdom_logger.py:189: error: unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:197: error: unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:242: error: unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:372: error: unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:375: error: unused "type: ignore" comment
+ ignite/contrib/handlers/visdom_logger.py:7: error: Cannot find implementation or library stub for module named "torch"  [import]
+ ignite/contrib/handlers/visdom_logger.py:8: error: Cannot find implementation or library stub for module named "torch.nn"  [import]
+ ignite/contrib/handlers/visdom_logger.py:9: error: Cannot find implementation or library stub for module named "torch.optim"  [import]
+ ignite/contrib/handlers/visdom_logger.py:152: error: Cannot find implementation or library stub for module named "visdom"  [import]
+ ignite/contrib/handlers/visdom_logger.py:189: error: unused "type: ignore" comment
+ ignite/contrib/handlers/visdom_logger.py:197: error: unused "type: ignore" comment
+ ignite/contrib/handlers/visdom_logger.py:242: error: unused "type: ignore" comment
+ ignite/contrib/handlers/visdom_logger.py:372: error: unused "type: ignore" comment
+ ignite/contrib/handlers/visdom_logger.py:375: error: unused "type: ignore" comment

anyio (https://github.com/agronholm/anyio.git)
+ src/anyio/abc/_sockets.py:67: error: Invalid index type "int" for "Dict[Union[socket, AddressFamily, Tuple[str, int], str], Callable[[], object]]"; expected type "Union[socket, AddressFamily, Tuple[str, int], str]"
+ src/anyio/abc/_sockets.py:70: error: Invalid index type "int" for "Dict[Union[socket, AddressFamily, Tuple[str, int], str], Callable[[], object]]"; expected type "Union[socket, AddressFamily, Tuple[str, int], str]"
+ src/anyio/_backends/_asyncio.py:1243: error: On Python 3 '{}'.format(b'abc') produces "b'abc'", not 'abc'; use '{!r}'.format(b'abc') if this is desired behavior
+ src/anyio/_backends/_asyncio.py:1303: error: Incompatible return value type (got "_socket.socket", expected "socket.socket")
+ src/anyio/_backends/_asyncio.py:1362: error: "socket" has no attribute "accept"

werkzeug (https://github.com/pallets/werkzeug.git)
+ src/werkzeug/serving.py:234: error: "socket" has no attribute "getpeercert"

@srittau srittau merged commit 6ee6748 into python:master May 30, 2021
@srittau srittau deleted the rework-socket branch May 30, 2021 18:17
agronholm pushed a commit to agronholm/anyio that referenced this pull request Jun 1, 2021
SocketType is an alias for the private `_socket.socket` class, a superclass of `socket.socket`. It is better to just always use `socket.socket` in types. See https://bugs.python.org/issue44261 and python/typeshed#5545 for some context.
lucas-six added a commit to lucas-six/python-cookbook that referenced this pull request Jul 25, 2022
lucas-six added a commit to lucas-six/python-cookbook that referenced this pull request Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants