Skip to content

Conversation

@AraHaan
Copy link
Collaborator

@AraHaan AraHaan commented Jan 9, 2025

This implements #40.

I also added in a NoArgs version as well because I used it as a means to test my own C extension and simply left that in for this feature as others might want to call an awaitable without any args and without needing to implement their own function to calling them.

@AraHaan AraHaan added the feature New feature or request label Jan 9, 2025
@AraHaan AraHaan added this to the 1.4.0 milestone Jan 9, 2025
@AraHaan AraHaan requested a review from ZeroIntensity January 9, 2025 22:40
@netlify
Copy link

netlify bot commented Jan 9, 2025

Deploy Preview for awaitable canceled.

Name Link
🔨 Latest commit 89a63bc
🔍 Latest deploy log https://app.netlify.com/sites/awaitable/deploys/67805f05db53870008b1949a

@AraHaan AraHaan force-pushed the additional-await-functions branch from 3f0de3c to 89a63bc Compare January 9, 2025 23:42
Copy link
Owner

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

I don't see the need for this. There's no upstream equivalent of the proposed PyAwaitable_AwaitFunctionKeywords, so I don't think we're at liberty to start re-inventing the call API here.

Regarding PyAwaitable_AwaitFunctionNoArgs, I originally added PyAwaitable_AwaitFunction because I thought it would cover all bases, but I'm thinking otherwise now. If we add NoArgs, then why not OneArg? Why not CallMethodNoArgs? And so on. It's just too much messy maintenance to save a few lines of code.

I plan to remove PyAwaitable_AwaitFunction in 2.0.0. We shouldn't lean towards more APIs that involve automatically registering the result to the awaitable :(

@AraHaan AraHaan deleted the additional-await-functions branch February 13, 2025 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants