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.

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

Add pydantic conversion compatibility with specialized list class #2909

Conversation

tjeerddie
Copy link
Contributor

Description

  • move _is_list check before the _is_generic check in StrawberryAnnotation.resolve, so it does not get stuck on ValueError: Not supported type and first checks if it is a list.
  • change StrawberryAnnotation._is_list to check if the annotation extends from list and can be considered a list.

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).

@botberry
Copy link
Member

botberry commented Jun 30, 2023

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Enhancements:

  • Improved pydantic conversion compatibility with specialized list classes.
    • Modified StrawberryAnnotation._is_list to check if the annotation extends from the list type, enabling it to be considered a list.
    • in StrawberryAnnotation Moved the _is_list check before the _is_generic check in resolve to avoid unsupported error in _is_generic before it checked _is_list.

This enhancement enables the usage of constrained lists as class types and allows the creation of specialized lists. The following example demonstrates this feature:

import strawberry
from pydantic import BaseModel, ConstrainedList


class FriendList(ConstrainedList):
    min_items = 1


class UserModel(BaseModel):
    age: int
    friend_names: FriendList[str]


@strawberry.experimental.pydantic.type(UserModel)
class User:
    age: strawberry.auto
    friend_names: strawberry.auto

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to tjeerddie for the PR 👏

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

@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Merging #2909 (e910059) into main (7adcbfe) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2909      +/-   ##
==========================================
- Coverage   96.53%   96.39%   -0.15%     
==========================================
  Files         468      468              
  Lines       29112    29169      +57     
  Branches     3582     3589       +7     
==========================================
+ Hits        28104    28118      +14     
- Misses        827      863      +36     
- Partials      181      188       +7     

@thejaminator
Copy link
Contributor

sorry i missed this, this seems quite cool and i'll check it out

@thejaminator thejaminator self-requested a review July 11, 2023 17:17
@thejaminator thejaminator self-assigned this Jul 11, 2023
@tjeerddie tjeerddie force-pushed the bug/pydantic-convert-specialized-list-class branch from 3059020 to d7bc31d Compare July 18, 2023 07:35
@tjeerddie tjeerddie force-pushed the bug/pydantic-convert-specialized-list-class branch from d7bc31d to ee2c43a Compare July 31, 2023 09:06
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 1, 2023

CodSpeed Performance Report

Merging #2909 will not alter performance

Comparing tjeerddie:bug/pydantic-convert-specialized-list-class (e910059) with main (7adcbfe)

Summary

✅ 12 untouched benchmarks

@patrick91 patrick91 self-requested a review August 1, 2023 09:05
@tjeerddie tjeerddie force-pushed the bug/pydantic-convert-specialized-list-class branch 2 times, most recently from 08139ba to 0787a32 Compare August 8, 2023 12:20
@patrick91
Copy link
Member

@tjeerddie thanks for keeping this up to date, I'll take a look at it this week

@tjeerddie tjeerddie force-pushed the bug/pydantic-convert-specialized-list-class branch from 0787a32 to 0272a25 Compare August 9, 2023 07:40
- Modified `StrawberryAnnotation._is_list` to check if the `annotation` extends from the `list` type, enabling it to be considered a list.
- in `StrawberryAnnotation` Moved the `_is_list` check before the `_is_generic` check in `resolve` to avoid `unsupported` error in `_is_generic` before it checked `_is_list`.
@tjeerddie tjeerddie force-pushed the bug/pydantic-convert-specialized-list-class branch from 0272a25 to e910059 Compare August 10, 2023 07:31
@patrick91 patrick91 merged commit fa71d81 into strawberry-graphql:main Aug 14, 2023
50 of 57 checks passed
etripier pushed a commit to Greenbax/strawberry that referenced this pull request Oct 25, 2023
…rawberry-graphql#2909)

* Add tests for pydantic conversion for contrained list

* Add pydantic conversion compatibility with specialized list class

- Modified `StrawberryAnnotation._is_list` to check if the `annotation` extends from the `list` type, enabling it to be considered a list.
- in `StrawberryAnnotation` Moved the `_is_list` check before the `_is_generic` check in `resolve` to avoid `unsupported` error in `_is_generic` before it checked `_is_list`.

* Add release file

---------

Co-authored-by: Tjeerd.Verschragen <tjeerd.verschragen@surf.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants