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

fix: DTOs for generic types #2512

Closed
wants to merge 3 commits into from
Closed

fix: DTOs for generic types #2512

wants to merge 3 commits into from

Conversation

peterschutt
Copy link
Contributor

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR
  • Pre-Commit Checks were ran and passed
  • Tests were ran and passed

Description

Close Issue(s)

Closes #2500

…is_subclass_of()

Using `is_class_and_subclass()` is dangerous, because it hides things. Simply returning `False` if a type isn't a class isn't necessarily what we want.

A good example of this is that one of the things that we needed to fix for this PR was to add `ClassVar` to our set of wrapper types. Just like `Required` et al., `ClassVar` isn't an alias for a builtin, it is a type that provides more information to type checkers. However, due to `is_class_and_subclass()` `FieldDefinition.is_subclass_of(int)` would return `True` for `Required[int]`, but `False` for `ClassVar[int]`. This is something that we would have caught earlier were it not for `is_class_and_subclass()`.

The other change required to get this through was to return `False` from `FieldDefinition.is_subclass_of()` if the type is a `Literal`. This is not correct, but is the same as current behavior, and I want to take some time to think about how we should handle these.
@peterschutt peterschutt force-pushed the dto-for-generic-types branch from 71548dd to 0b591b6 Compare October 24, 2023 13:28
@peterschutt peterschutt force-pushed the dto-for-generic-types branch from 0b591b6 to af7ba5d Compare October 24, 2023 13:36
@provinzkraut provinzkraut deleted the dto-for-generic-types branch November 19, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Support generic types in DTOs
1 participant