Skip to content

Changed the semantics of Annotated. #893

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

Closed
mvaled opened this issue Mar 28, 2022 · 1 comment · Fixed by #925
Closed

Changed the semantics of Annotated. #893

mvaled opened this issue Mar 28, 2022 · 1 comment · Fixed by #925
Labels
bug Something isn't working

Comments

@mvaled
Copy link
Contributor

mvaled commented Mar 28, 2022

Bug report

What's wrong

typing.Annotated is being reported incorrectly by changing their semantics. I have a small dataclass like:

@dataclass(unsafe_hash=True)
class RatingComposite(LoweringComposite):
    require_picture: Annotated[bool, ExtendedBooleanType(labels={True: "Always", False: "Never")] = True
    max_value: Annotated[int, IntegerType(min_value=1, max_value=10)] = 5

We're using xotl.plato to parse and dump this kind of structures to JSON. However, this code fails to type-check with:

error: Invalid type comment or annotation
note: Suggestion: use ExtendedBooleanType[...] instead of ExtendedBooleanType(...)
error: Incompatible types in assignment (expression has type "bool", variable has type "WithAnnotations[builtins.bool]")

error: Invalid type comment or annotation
note: Suggestion: use IntegerType[...] instead of IntegerType(...)
error: Incompatible types in assignment (expression has type "int", variable has type "WithAnnotations[builtins.int]")

Without django-stubs-ext mypy would accept that annotations and their default values.

How is that should be

  • The extension should not impose any syntax to the annotation. The purpose of Annotated is to annotate with 3rd party objects and django-stub-ext should not change the semantics of it. If it doesn't know about ExtendedBooleanType or IntegerType, it should completly ignore them. Notice the second example is quite similar to the use cases described in PEP 593.

  • The final erased type WithAnnotations[builtins.bool] should only be introduced for the cases django-stub-ext knows about; in this case I'd expect the final type would be just bool.

System information

  • OS: Linux (mostly, Manjaro and also Debian-based containers in a CI pipeline)
  • python version: 3.10.2
  • django version: 4.0.3 (but is actually not Django related code)
  • mypy version: 0.941
  • django-stubs version: 1.9.0
  • django-stubs-ext version: 0.3.1
@mvaled mvaled added the bug Something isn't working label Mar 28, 2022
@g-as
Copy link
Contributor

g-as commented Apr 14, 2022

Same here, with a pydantic model.

sobolevn pushed a commit that referenced this issue Apr 28, 2022
* Fix stubs related to `(Async)RequestFactory` and `(Async)Client`

* Revert incorrect removal.

* Allow set as `unique_together`, use shared type alias.

* Revert `Q.__init__` to use only `*args, **kwargs` to remove false-positive with `Q(**{...})`

* Add abstract methods to `HttpResponseBase` to create common interface.

* Remove monkey-patched attributes from `HttpResponseBase` subclasses.

* Add QueryDict mutability checks (+ plugin support)

* Fix lint

* Return back GenericForeignKey to `Options.get_fields`

* Minor fixup

* Make plugin code typecheck with `--warn-unreachable`, minor performance increase.

* Better types for `{unique, index}_together` and Options.

* Fix odd type of `URLResolver.urlconf_name` which isn't a str actually.

* Better types for field migration operations.

* Revert form.files to `MultiValueDict[str, UploadedFile]`

* Compatibility fix (#916)

* Do not assume that `Annotated` is always related to django-stubs (fixes #893)

* Restrict `FormView.get_form` return type to `_FormT` (class type argument). Now it is resolved to `form_class` argument if present, but also errors if it is not subclass of _FormT

* Fix CI (make test runnable on 3.8)

* Fix CI (make test runnable on 3.8 _again_)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants