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

Fix psycopg2 #10630

Merged
merged 2 commits into from
Sep 13, 2023
Merged

Fix psycopg2 #10630

merged 2 commits into from
Sep 13, 2023

Conversation

hamdanal
Copy link
Contributor

@hamdanal hamdanal commented Aug 29, 2023

Unblocks sbdchd/django-types#74

I did some careful reading of the docs and the C code and some testing in the repl. This hopefully yields a much better quality stubs for psycopg2.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Thanks and I'm sorry for the late review. Looks good to me, a few nits below.

stubs/psycopg2/psycopg2/__init__.pyi Show resolved Hide resolved
stubs/psycopg2/psycopg2/_psycopg.pyi Show resolved Hide resolved
stubs/psycopg2/psycopg2/_psycopg.pyi Show resolved Hide resolved
stubs/psycopg2/psycopg2/_psycopg.pyi Show resolved Hide resolved
stubs/psycopg2/psycopg2/_range.pyi Outdated Show resolved Hide resolved
stubs/psycopg2/psycopg2/_range.pyi Outdated Show resolved Hide resolved
@hamdanal
Copy link
Contributor Author

Running stubtest for psycopg2 locally fails on my machine because it tries to build it from source and apparently it is missing some dependencies. psycopg2 is special because the compiled wheels exist on PyPI as the psycopg2-binary package. I did this locally to get around this issue:

diff --git a/tests/stubtest_third_party.py b/tests/stubtest_third_party.py
index af393c35f..65241e7be 100755
--- a/tests/stubtest_third_party.py
+++ b/tests/stubtest_third_party.py
@@ -45,7 +45,10 @@ def run_stubtest(
             print_error("fail")
             raise
         dist_extras = ", ".join(stubtest_settings.extras)
-        dist_req = f"{dist_name}[{dist_extras}]=={metadata.version}"
+        pypi_dist_name = dist_name
+        if dist_name == "psycopg2":
+            pypi_dist_name = "psycopg2-binary"  # psycopg2 is named psycopg2-binary on PyPI
+        dist_req = f"{pypi_dist_name}[{dist_extras}]=={metadata.version}"

         # If tool.stubtest.stubtest_requirements exists, run "pip install" on it.
         if stubtest_settings.stubtest_requirements:

I don't know how many packages you have that are in the same situation but installing from wheels would make CI faster and cause less problems while developing. It might not be worth the effort though, I don't know.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit 71a35b4 into python:main Sep 13, 2023
def format(self, *args, **kwargs) -> Composed: ...
def join(self, seq) -> Composed: ...
def format(self, *args: Composable, **kwargs: Composable) -> Composed: ...
def join(self, seq: SupportsIter[Composable]) -> Composed: ...
Copy link
Contributor

@andersk andersk Sep 15, 2023

Choose a reason for hiding this comment

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

This needs to be Iterable[Composable], not SupportsIter[Composable]. The difference is that

  • seq: Iterable[Composable] means iter(seq): Iterator[Composable] (hence next(iter(seq)): Composable), while
  • seq: SupportsIter[Composable] means iter(seq): Composable, which is not an iterator.

andersk added a commit to andersk/django-stubs that referenced this pull request Sep 15, 2023
An implementor of _Composable is required to support + with its own
type, not with an arbitrary other implementor of _Composable.  This is
now required for compatibility with
python/typeshed#10630.

Signed-off-by: Anders Kaseorg <[email protected]>
sobolevn pushed a commit to typeddjango/django-stubs that referenced this pull request Sep 15, 2023
…#1714)

An implementor of _Composable is required to support + with its own
type, not with an arbitrary other implementor of _Composable.  This is
now required for compatibility with
python/typeshed#10630.

Signed-off-by: Anders Kaseorg <[email protected]>
@hamdanal hamdanal deleted the psycopg2 branch September 16, 2023 07:05
hamdanal added a commit to hamdanal/typeshed that referenced this pull request Oct 28, 2023
This is an error I introduced in python#10630 because I didn't know protocols
need to be explicitly inherited from in other protocol subclasses.

The added test shows the change. Basically these protocols were unusable.
AlexWaygood pushed a commit that referenced this pull request Oct 28, 2023
This is an error I introduced in #10630 because I didn't know protocols
need to be explicitly inherited from in other protocol subclasses.

The added test shows the change. Basically these protocols were unusable.
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.

3 participants