Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-97696 Remove unnecessary check for eager_start kwarg#104188

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

jbower-fb
Copy link
Contributor

Part ofgh-97696 in response to comments on#102853.

@jbower-fbjbower-fb changed the titleRemove unnecessary check for eager_start kwarg gh-97696 Remove unnecessary check for eager_start kwargMay 5, 2023
@arhadthedevarhadthedev added skip news stdlibPython modules in the Lib dir labelsMay 5, 2023
@gvanrossumgvanrossum removed their request for reviewMay 5, 2023 16:38
Copy link
Contributor

@itamaroitamaro left a comment

Choose a reason for hiding this comment

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

this trades off a clear and upfront (as soon as the factory is set) error:

TypeError: Provided constructor does not support eager task execution

with a delayed (until the first task creation) and less clear error:

TypeError: AsyncTree.run.<locals>.loop_factory.<locals>.counting_task_constructor() got an unexpected keyword argument 'eager_start'

I think the "less clear" error is still clear enough to be actionable, and I wasn't crazy about that use of inspect and the fact that it rejects ctors that take and pass down**kwargs, so in balance I think this change is good.

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

How about instead we add a docstring explaining the requirement?

@carljm
Copy link
Member

How about instead we add a docstring explaining the requirement?

cf also@kumaraditya303 's observation in#97696 (comment) that the neweager_start argument to theTask constructor needs to be documented, and perhapscreate_eager_task_factory could generally use stronger documentation with an example.

@jbower-fb
Copy link
ContributorAuthor

To get us started, I've added a possible docstring forcreate_eager_task_factory().

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

I have some suggestions for the docstring.

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

Who's waiting for whom here?

Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

Great -- will merge once tests pass!

jbower-fb reacted with thumbs up emoji
@gvanrossumgvanrossumenabled auto-merge (squash)May 9, 2023 00:22
auto-merge was automatically disabledMay 9, 2023 00:25

Head branch was pushed to by a user without write access

@jbower-fb
Copy link
ContributorAuthor

Thanks for the review, took me a few commits to tidy things up though. I hope it's good to go now.

@gvanrossumgvanrossumenabled auto-merge (squash)May 9, 2023 00:33
@gvanrossumgvanrossum merged commitbf89d42 intopython:mainMay 9, 2023
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (47 commits)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)pythongh-103193: Improve `getattr_static` test coverage (python#104286)  Trim trailing whitespace and test on CI (python#104275)pythongh-102500: Remove mention of bytes shorthand (python#104281)pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)pythongh-104273: Remove redundant len() calls in argparse function (python#104274)pythongh-64660: Don't hardcode Argument Clinic return converter result variable name (python#104200)pythongh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (python#104266)pythonGH-102613: Improve performance of `pathlib.Path.rglob()` (pythonGH-104244)pythongh-103650: Fix perf maps address format (python#103651)pythonGH-89812: Churn `pathlib.Path` methods (pythonGH-104243)  ...
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (29 commits)pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)  require-pr-label.yml: Add missing "permissions:" (python#104309)pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)pythongh-103193: Improve `getattr_static` test coverage (python#104286)  Trim trailing whitespace and test on CI (python#104275)pythongh-102500: Remove mention of bytes shorthand (python#104281)pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)  ...
@jbower-fbjbower-fb deleted the removeeagerstartintrospect branchMay 9, 2023 17:08
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (156 commits)pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)pythongh-104240: return code unit metadata from codegen (python#104300)pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)  require-pr-label.yml: Add missing "permissions:" (python#104309)pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)  ...
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (35 commits)pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)pythongh-104240: return code unit metadata from codegen (python#104300)pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)  require-pr-label.yml: Add missing "permissions:" (python#104309)pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)  ...
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@carljmcarljmcarljm left review comments

@itamaroitamaroitamaro approved these changes

@gvanrossumgvanrossumgvanrossum approved these changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303kumaraditya303 is a code owner

@willingcwillingcAwaiting requested review from willingcwillingc is a code owner

Assignees
No one assigned
Labels
skip newsstdlibPython modules in the Lib dirtopic-asyncio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@jbower-fb@carljm@itamaro@gvanrossum@arhadthedev@gaogaotiantian@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp