Update `exit` parameters in stubs#9696

Avasam · 2023-02-09T04:14:14Z

Follow-up to #9519 for third-party stubs now that a new version of mypy has been released
Ref #9297

I could not validate psycopg2, hdbcli and _cffi_backend so I left those as-is

srittau · 2023-02-09T07:38:11Z

I'm not sure that replacing proper __exit__ types with (essentially) object is the right course of action. I can't imagine a situation where you would want to pass some arbitrary into __exit__. In fact I'd say that's a sign of potential bug (even if __exit__ itself accepts it.) It also makes sub-classing more difficult, as an overridden __exit__ would also need to accept object, instead of the proper types.

…/typeshed into __exit__-parameters-stubs

Avasam · 2023-02-09T15:28:29Z

[...] I can't imagine a situation where you would want to pass some arbitrary into __exit__. In fact I'd say that's a sign of potential bug (even if __exit__ itself accepts it.)
It also makes sub-classing more difficult, as an overridden __exit__ would also need to accept object, instead of the proper types.

Should non-star __a?exit__ parameters always use their exact type then? (even if unused). Maybe except for mock.

srittau · 2023-02-09T15:35:01Z

I'd say ideally all __exit__ signatures would use the following signature (with the exception of the return type), even those that just use *args in the implementation:

_E=TypeVar("_E", bound=BaseException) classX: @overloaddef__exit__(self, __type: None, __value: None, __exc: None) ->None: ... @overloaddef__exit__(self, __type: type[_E], __value: type[_E], __exc: TracebackType) ->None: ...

But since that is unwieldy, we can use the best approximation in a current situation. So I'm actually fine to use Unused for new annotations if that's the fastest way to do it, but we shouldn't regress back from the ideal situation. But I'd be interested in what the other maintainers think.

Avasam · 2023-02-09T15:49:00Z

[...] So I'm actually fine to use Unused for new annotations if that's the fastest way to do it, but we shouldn't regress back from the ideal situation.

Ok, I think my last commit satisfies your comment. (now the PR only adds new Unused when it's unused, new object for *args params and changes *args: object to *args: Unused when unused.

I'd say ideally all __exit__ signatures would use the following signature (with the exception of the return type), even those that just use *args in the implementation: [...]
But since that is unwieldy, we can use the best approximation in a current situation.

Given this isn't a "stopgap", "urgent" or "in the meantime" PR. It's just splitting off a would-be bigger PR about semantically using Unused instead of object or _Unused for unused parameters.
Since it's focusing entirely on exit dunder methods. I don't mind changing it to do it "the proper way" everywhere (and even documenting in CONTRIBUTING.md, and/or update Flake8-pyi, any decision about typing exit dunder methods)

srittau · 2023-02-09T16:15:49Z

Side note: Changing __exit__ to overloads could be disruptive or could even now work. I'd definitely would test it with a small explorative PR first.

JelleZijlstra · 2023-02-22T07:39:16Z

This PR has a lot of conflicts now.

I'd be hesitant to use the overloaded signature @srittau suggests, though it is probably technically correct:

There may be rare instances where only some of the arguments are None, though recent changes to CPython make this less and less likely.
Using this signature will force users writing subclasses to also write the verbose signature with overloads, which feels like unnecessary busywork.

Avasam · 2023-02-22T17:56:24Z

This PR has a lot of conflicts now.

Expected, but not hard to fix :)

About the signature, should I leave it a-is, (can be done in a different PR and/or discussed further). At least get someway there? Or full-on change all signatures?

…__-parameters-stubs

AlexWaygood · 2023-02-24T15:13:33Z

My preference would be to generally type __(a)exit__ methods like this, if the method uses *args at runtime:

def__exit__(self, *args: Unused) ->None: ...

And to generally type __(a)exit__ methods like this, if the method includes all three parameters explicitly at runtime:

def__exit__(self, exc_typ: type[BaseException] |None, exc_val: BaseException|None, exc_tb: TracebackType|None

(There will obviously be exceptions to these rules where doing something else makes sense.)

I agree with @srittau that using object or Unused for annotations with the three-argument form could make subclassing unnecessarily difficult, as subclasses will be obliged to use object for annotating those parameters in the subclass.

I also agree with @srittau that a truly precise signature for __(a)exit__ methods would require overloads, but that this would be pretty tedious to do everywhere, and probably wouldn't actually improve type safety significantly. And I agree with @JelleZijlstra that using overloads could also make it harder for people to subclass these classes, so has downsides as well.

Avasam · 2023-02-25T19:11:52Z

And to generally type __(a)exit__ methods like this, if the method includes all three parameters explicitly at runtime:
def__exit__(self, exc_typ: type[BaseException] |None, exc_val: BaseException|None, exc_tb: TracebackType|None

I know you said there could be exceptions. But that sounds like something Flake8-PYI could check for and/or add in its existing __(a)exit__ checks, if the exceptions are rare enough.

…/typeshed into __exit__-parameters-stubs

AlexWaygood · 2023-02-25T19:18:31Z

I know you said there could be exceptions. But that sounds like something Flake8-PYI could check for and/or add in its existing __(a)exit__ checks, if the exceptions are rare enough.

Y036 actually used to be stricter, but Jelle persuaded me to relax it slightly in PyCQA/flake8-pyi#209 after we came across a situation in #7625 (comment) where there was a good case for doing something different. Maybe the exceptions that break the rule are rare enough that we could consider reverting that change... idk, I don't really have a strong opinion on exactly how strict flake8-pyi should be on this ¯\_(ツ)_/¯

Avasam · 2023-02-25T19:20:33Z

stubs/caldav/caldav/davclient.pyi

 def__enter__(self) ->Self: ...
-def__exit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+def__exit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:20:47Z

stubs/influxdb-client/influxdb_client/client/influxdb_client.pyi

 def__enter__(self) ->Self: ...
-def__exit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+def__exit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:20:59Z

stubs/influxdb-client/influxdb_client/client/influxdb_client_async.pyi

 asyncdef__aenter__(self) ->Self: ...
-asyncdef__aexit__(self, exc_type: object, exc: object, tb: object) ->None: ...
+asyncdef__aexit__(
+self, exc_type: type[BaseException] |None, exc: BaseException|None, tb: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:21:17Z

stubs/pika/pika/adapters/blocking_connection.pyi

 def__enter__(self) ->Self: ...
-def__exit__(self, exc_type: object, value: object, traceback: object) ->None: ...
+def__exit__(
+self, exc_type: type[BaseException] |None, value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:21:37Z

stubs/psycopg2/psycopg2/_psycopg.pyi

 defxid(self, format_id, gtrid, bqual) ->Xid: ...
 def__enter__(self) ->Self: ...
-def__exit__(self, __type: object, __name: object, __tb: object) ->None: ...
+def__exit__(self, __type: type[BaseException] |None, __name: BaseException|None, __tb: TracebackType|None) ->None: ...


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:21:42Z

stubs/pyserial/serial/threaded/__init__.pyi

 def__enter__(self) ->Protocol: ...
-def__exit__(self, __exc_type: object, __exc_val: object, __exc_tb: object) ->None: ...
+def__exit__(
+self, __exc_type: type[BaseException] |None, __exc_val: BaseException|None, __exc_tb: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:22:12Z

stubs/redis/redis/asyncio/cluster.pyi

 asyncdef__aenter__(self) ->Self: ...
-asyncdef__aexit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+asyncdef__aexit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it still makes sense.

Avasam · 2023-02-25T19:22:47Z

stubs/tqdm/tqdm/std.pyi

 def__enter__(self) ->Self: ...
-def__exit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+def__exit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:23:02Z

stubs/redis/redis/asyncio/utils.pyi

 asyncdef__aenter__(self) ->Pipeline[_StrType]: ...
-asyncdef__aexit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+asyncdef__aexit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:23:09Z

stubs/redis/redis/asyncio/cluster.pyi

 asyncdef__aenter__(self) ->Self: ...
-asyncdef__aexit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+asyncdef__aexit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:23:13Z

stubs/redis/redis/asyncio/cluster.pyi

 def__enter__(self) ->Self: ...
-def__exit__(self, exc_type: object, exc_value: object, traceback: object) ->None: ...
+def__exit__(
+self, exc_type: type[BaseException] |None, exc_value: BaseException|None, traceback: TracebackType|None


This used to be typed with objects. Please validate it's still correct.

Avasam · 2023-02-25T19:24:29Z

[...] I don't really have a strong opinion on exactly how strict flake8-pyi should be on this ¯\_(ツ)_/¯

Me neither, just throwing suggestions :)

Just in case, I flagged all the 3 parameters definitions that used to all be objects.

AlexWaygood

All the places you flagged look fine -- just one suggestion

stubs/humanfriendly/humanfriendly/testing.pyi

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood

Thanks!

github-actions · 2023-02-25T21:48:07Z

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

Avasam added 2 commits February 8, 2023 23:09

Update __exit__ parameters in stubs
f35b8e9

Abstract startablecontext means not unused
2e8cba4

This comment has been minimized.
Sign in to view

Avasam added 4 commits February 9, 2023 09:50

Restore stubs/SQLAlchemy/sqlalchemy/engine/base.pyi
71740e1

Revert changing already defined non-object exit params
56e7ad9

Merge branch '__exit__-parameters-stubs' ofhttps://github.com/Avasam…
f6cd777
…/typeshed into __exit__-parameters-stubs

Merge branch 'main' into __exit__-parameters-stubs
b5cb959

This comment has been minimized.
Sign in to view

Avasam mentioned this pull request Feb 10, 2023
Update Unused parameters in stubs/#9704
Merged

Avasam added 2 commits February 23, 2023 03:50

Merge branch 'main' ofhttps://github.com/python/typeshedinto __exit…
6d966c8
…__-parameters-stubs

.
6c733c3

This comment has been minimized.
Sign in to view

Merge branch 'main' into __exit__-parameters-stubs
2363a88

This comment has been minimized.
Sign in to view

AlexWaygoodand others added 2 commits February 24, 2023 23:58

Merge branch 'main' into __exit__-parameters-stubs
c75545e

[pre-commit.ci] auto fixes from pre-commit.com hooks
21af467

This comment has been minimized.
Sign in to view

Avasam added 2 commits February 25, 2023 14:12

Apply PR suggestions
631e414

Merge branch '__exit__-parameters-stubs' ofhttps://github.com/Avasam…
272e50f
…/typeshed into __exit__-parameters-stubs

post merge
249b539

Avasam commented Feb 25, 2023
View reviewed changes

This comment has been minimized.
Sign in to view

AlexWaygood reviewed Feb 25, 2023
View reviewed changes

stubs/humanfriendly/humanfriendly/testing.pyi Outdated Show resolvedHide resolved

Update stubs/humanfriendly/humanfriendly/testing.pyi
7acd4cb
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood approved these changes Feb 25, 2023
View reviewed changes

AlexWaygood merged commit 52ec44f into python:mainFeb 25, 2023

Avasam deleted the __exit__-parameters-stubs branch February 25, 2023 22:00

Uh oh!

Update __exit__ parameters in stubs#9696

Update __exit__ parameters in stubs #9696

Uh oh!

Conversation

Avasam commented Feb 9, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

srittau commented Feb 9, 2023

Uh oh!

Avasam commented Feb 9, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srittau commented Feb 9, 2023

Uh oh!

This comment has been minimized.

Avasam commented Feb 9, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srittau commented Feb 9, 2023

Uh oh!

JelleZijlstra commented Feb 22, 2023

Uh oh!

Avasam commented Feb 22, 2023

Uh oh!

This comment has been minimized.

AlexWaygood commented Feb 24, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

Avasam commented Feb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Feb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AvasamFeb 25, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Update `exit` parameters in stubs#9696

Update `exit` parameters in stubs #9696

Avasam commented Feb 9, 2023•
edited
Loading

Avasam commented Feb 9, 2023•
edited
Loading

Avasam commented Feb 9, 2023•
edited
Loading

Avasam commented Feb 25, 2023•
edited
Loading

AlexWaygood commented Feb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

AvasamFeb 25, 2023•
edited
Loading

Avasam commented Feb 25, 2023•
edited
Loading