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.

Sign up for GitHub

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

Jump to bottom

Add is_auto utility #1721

Merged
merged 3 commits into from
Mar 28, 2022
Merged

Add is_auto utility #1721

merged 3 commits into from
Mar 28, 2022

Conversation

bellini666
Copy link
Member

@bellini666 bellini666 commented Mar 13, 2022 •
edited
Loading

Add is_auto utility for checking if a type is strawberry.auto,
considering the possibility of it being a StrawberryAnnotation or
even being used inside Annotated.

Description

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@bellini666 bellini666 self-assigned this Mar 13, 2022
@codecov
Copy link

codecov bot commented Mar 13, 2022 •
edited
Loading

Codecov Report

Merging #1721 (cd9208e) into main (3824c7d) will decrease coverage by 0.03%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##             main    #1721      +/-   ##
==========================================
- Coverage   98.13%   98.10%   -0.04%     
==========================================
  Files         139      139              
  Lines        5214     5234      +20     
  Branches      947      954       +7     
==========================================
+ Hits         5117     5135      +18     
  Misses         53       53              
- Partials       44       46       +2

@botberry
Copy link
Member

botberry commented Mar 13, 2022 •
edited
Loading

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Add is_auto utility for checking if a type is strawberry.auto,
considering the possibility of it being a StrawberryAnnotation or
even being used inside Annotated.

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Thiago Bellini Ribeiro for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

patrick91
patrick91 reviewed Mar 14, 2022
View reviewed changes
strawberry/auto.py Outdated Show resolved Hide resolved
BryceBeagle
BryceBeagle suggested changes Mar 20, 2022
View reviewed changes
strawberry/auto.py Outdated
@@ -21,3 +25,25 @@ def __repr__(self):


auto: Final = Annotated[Any, StrawberryAuto()]


def is_auto(type_: Union[StrawberryAnnotation, StrawberryType, type]):
Copy link
Member

Choose a reason for hiding this comment

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

Can is_auto be a classmethod (or staticmethod) on StrawberryAuto instead of an unbound function?

Copy link
Member

Choose a reason for hiding this comment

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

In fact, could we just implement StrawberryAuto.__instancecheck__?

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome idea! I went the same road for UNSET/is_unset here, but maybe we should change it too in the future?

Btw, its done!

Copy link
Member

Choose a reason for hiding this comment

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

Yep definitely think we should do that in the future 👍. Personally, I don't think unbound functions are good practice in Python in anything other than simple scripts.

BryceBeagle
BryceBeagle approved these changes Mar 20, 2022
View reviewed changes
Copy link
Member

@BryceBeagle BryceBeagle left a comment

Choose a reason for hiding this comment

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

Minor requests; otherwise lgtm.

Will approve now so that either you or Patrick can merge

strawberry/auto.py Outdated Show resolved Hide resolved
strawberry/auto.py Outdated Show resolved Hide resolved
strawberry/experimental/pydantic/object_type.py Outdated Show resolved Hide resolved
@bellini666
Add is_auto utility
6322e18 
Add `is_auto` utility for checking if a type is `strawberry.auto`,
considering the possibility of it being a `StrawberryAnnotation` or
even being used inside `Annotated`.
@bellini666
Add type hints to is_auto checker
2497c2b
@bellini666
Copy link
Member Author

Hey @BryceBeagle and @patrick91 , sorry for the delay here... I have not been able to properly focus on some contributions lately.

Just fixed the suggestions made by @BryceBeagle . This PR is already approved, but do you want to take another look at it?

@bellini666 bellini666 merged commit fc24f62 into main Mar 28, 2022
@bellini666 bellini666 deleted the is_auto branch March 28, 2022 01:10
msobas pushed a commit to msobas/strawberry that referenced this pull request Apr 10, 2022
@bellini666
Add is_auto utility (strawberry-graphql#1721)
603acaf 
* Add is_auto utility

Add `is_auto` utility for checking if a type is `strawberry.auto`,
considering the possibility of it being a `StrawberryAnnotation` or
even being used inside `Annotated`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BryceBeagle BryceBeagle BryceBeagle approved these changes

@patrick91 patrick91 Awaiting requested review from patrick91

Assignees

@bellini666 bellini666

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bellini666 @botberry @patrick91 @BryceBeagle