Skip to content

Conversation

MovisLi
Copy link
Contributor

@MovisLi MovisLi commented Sep 10, 2024

Change Summary

Add judgement for validate_call decorator. Now we can get True return from inpect.iscoroutinefunction().

Related issue number

fix #10370

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @sydney-runkle

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Sep 10, 2024
@MovisLi
Copy link
Contributor Author

MovisLi commented Sep 10, 2024

@sydney-runkle , please review.

Copy link

codspeed-hq bot commented Sep 10, 2024

CodSpeed Performance Report

Merging #10374 will not alter performance

Comparing MovisLi:validate_call_patch (d152715) with main (8b6d5fc)

Summary

✅ 49 untouched benchmarks

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  validate_call_decorator.py
Project Total  

This report was generated by python-coverage-comment-action

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Looking good, could you please add a test as well? Thanks!

@pydantic-hooky pydantic-hooky bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Sep 11, 2024
@pydantic-hooky pydantic-hooky bot assigned MovisLi and unassigned sydney-runkle Sep 11, 2024
@MovisLi
Copy link
Contributor Author

MovisLi commented Sep 11, 2024

Looking good, could you please add a test as well? Thanks!

I don't quite understand what changes need to be made to the test cases. I add some codes to check if it is a coroutine the async function under decorator. Please check it, thank you!

@Viicos
Copy link
Member

Viicos commented Sep 12, 2024

I don't quite understand what changes need to be made to the test cases. I add some codes to check if it is a coroutine the async function under decorator. Please check it, thank you!

Something like:

import inspect

@validate_call
async def test():
    pass

assert inspect.iscoroutinefunction(test)

@Viicos Viicos changed the title fix validate_call decorator #10370 Make sure inspect.iscoroutinefunction works on coroutines decorated with @validate_call Sep 12, 2024
@sydney-runkle
Copy link
Contributor

Happy to do another review here after we:

  • revert formatting changes
  • add a test like @Viicos sugested
  • Move the function definitions into the conditional blocks, see below:

currently:

# function definitions

if condition:
    return func1
else:
    return func2

desired:

if condition:
    # define func1
    return func1
else:
    # define func2
    return func2

Thanks, great work here!

@MovisLi
Copy link
Contributor Author

MovisLi commented Sep 12, 2024

@Viicos @sydney-runkle I finish all @sydney-runkle mentioned. Please check it, thank you!

Copy link
Contributor Author

@MovisLi MovisLi left a comment

Choose a reason for hiding this comment

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

check

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Awesome job, thanks for the help here!!

@sydney-runkle sydney-runkle merged commit 1cb0a8b into pydantic:main Sep 12, 2024
60 checks passed
@MovisLi MovisLi deleted the validate_call_patch branch September 12, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting author revision awaiting changes from the PR author relnotes-fix Used for bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support async functions with @validate_call decorator
3 participants