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

WithAnnotations Crash in 1.9.0 #760

Closed
lfrodrigues opened this issue Nov 28, 2021 · 35 comments · Fixed by #2319
Closed

WithAnnotations Crash in 1.9.0 #760

lfrodrigues opened this issue Nov 28, 2021 · 35 comments · Fixed by #2319
Labels
bug Something isn't working crash "Internal error" crashes from mypy mypy-plugin Issues specific to mypy_django_plugin

Comments

@lfrodrigues
Copy link

lfrodrigues commented Nov 28, 2021

Bug report

I have upgraded to 1.9.0 and I'm getting this crash when I run mypy . inside my src folder

Traceback (most recent call last):
  File "/home/parallels/Development/quickcheck/vitualenv/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/parallels/Development/quickcheck/vitualenv/lib/python3.9/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "mypy/main.py", line 87, in main
  File "mypy/main.py", line 165, in run_build
  File "mypy/build.py", line 179, in build
  File "mypy/build.py", line 254, in _build
  File "mypy/build.py", line 2697, in dispatch
  File "mypy/build.py", line 3014, in process_graph
  File "mypy/build.py", line 3092, in process_fresh_modules
  File "mypy/build.py", line 1991, in fix_cross_refs
  File "mypy/fixup.py", line 26, in fixup_module
  File "mypy/fixup.py", line 90, in visit_symbol_table
  File "mypy/fixup.py", line 46, in visit_type_info
  File "mypy/fixup.py", line 92, in visit_symbol_table
  File "mypy/nodes.py", line 885, in accept
  File "mypy/fixup.py", line 137, in visit_var
  File "mypy/types.py", line 846, in accept
  File "mypy/fixup.py", line 161, in visit_instance
  File "mypy/types.py", line 846, in accept
  File "mypy/fixup.py", line 154, in visit_instance
  File "mypy/fixup.py", line 269, in lookup_qualified_typeinfo
  File "mypy/fixup.py", line 297, in lookup_qualified
  File "mypy/fixup.py", line 306, in lookup_qualified_stnode
  File "mypy/lookup.py", line 47, in lookup_fully_qualified
AssertionError: Cannot find component 'WithAnnotations[admin' for "django_stubs_ext.WithAnnotations[admin.models.MyModel, TypedDict({'count_loan_id': Any})]"

The crash is generated by this piece of code.

self.var1 = (
    MyModel.objects.filter(
        to_process_date__gte=timezone.localtime() - timedelta(days=5),
        last_attempted_datetime__isnull=False,
    )
    .values('loan_id')
    .annotate(count_loan_id=Count('loan_id'))
    .filter(count_loan_id__gte=3)
    .values_list('loan_id', flat=True)
)

When typing mypy doesn't crash

self.var1: 'QuerySet[Any]' = ...

System information

  • OS:
  • python version: 3.9.2
  • django version: 2.2.17
  • mypy version: 0.910
  • django-stubs version: 1.9.0
  • django-stubs-ext version: 0.3.1
@lfrodrigues lfrodrigues added the bug Something isn't working label Nov 28, 2021
@AustinScola
Copy link

I'm seeing this issue too. It seems that the first run if there is no cache works, and subsequent runs work if files have changed. But if nothing has changed then I see this error.

Is there any plan to fix this?

@AustinScola
Copy link

Maybe related to #725?

@AustinScola
Copy link

This still seems to be a huge problem for me and preventing me from using the mypy cache.

@AdrienLemaire
Copy link

AdrienLemaire commented Jun 29, 2022

Same problem with django-stubs 1.12.0. I need to run mypy --no-incremental all the time.

@Pixel-Jack
Copy link

After removing the .mypy_cache directory it worked again

@intgr
Copy link
Collaborator

intgr commented Aug 19, 2022

I'm also having this issue.

I noticed that this error occurs when using --follow-imports=silent, but it disappears if I use --follow-imports=normal.

Edit: I was wrong, the effect is only temporary. --no-incremental is the actual fix.

@ashkop
Copy link

ashkop commented Nov 7, 2022

Still having this issue in 1.13.0.

@intgr intgr added crash "Internal error" crashes from mypy mypy-plugin Issues specific to mypy_django_plugin labels Nov 11, 2022
@tony
Copy link
Contributor

tony commented Nov 11, 2022

I seem to still get this with django-stubs 0.13.0 and mypy 0.990, python 3.10.1

@christianbundy
Copy link
Contributor

Do we have a way to consistently reproduce this issue? I was pretty sure that my codebase wasn't impacted by it, because I couldn't reproduce it locally, but after modifying my CI to use the cache I was able to immediately reproduce it when the cache crossed between two branches.

Can we reproduce it with rm -rf .mypy_cache && mypy && mypy, or does it only happen when files change between the cache generation and the final run?

@henribru
Copy link
Contributor

Can we reproduce it with rm -rf .mypy_cache && mypy && mypy, or does it only happen when files change between the cache generation and the final run?

I'm running into this issue, but it doesn't appear to happen with no changes between runs

@intgr
Copy link
Collaborator

intgr commented Jan 24, 2023

Do we have a way to consistently reproduce this issue?

Not that I know of. I could try to reduce this down to a minimal example if you're interested in debugging this.

@flaeppe
Copy link
Member

flaeppe commented Jan 24, 2023

I'd guess that most of the people here are running in to a caching issue as the plugin doesn't write WithAnnotations to cache:

annotated_typeinfo = helpers.add_new_class_for_module(
model_module_file,
type_name,
bases=[model_type] if fields_dict is not None else [model_type, annotated_model_type],
fields=fields_dict.items if fields_dict is not None else None,
no_serialize=True,
)

Which would be the no_serialize=True controlling it.

Now, if one attempts to go with no_serialize=False and start writing to cache. It opens up a whole new can of worms -> #700

WithAnnotations introduced here: #398
Attempted fix for #700: #725
Caching removed here: #881

When mypy tries to load WithAnnotations from cache, while the plugin is running with no_serialize=True, I'm suspecting it results in a traceback looking like:

@flaeppe
Copy link
Member

flaeppe commented Jan 24, 2023

I'm only guessing now, but a way to reproduce, manually, could perhaps be to have 2 files. Where file 1 would e.g. declare a function signature including WithAnnotations. Then file 2 would import and call the function(e.g. x = func()) from file 1.

Steps:

  • rm -r .mypy_cache
  • mypy .
  • Taint mypy cache of file 2 by editing and saving it (editing anything, anywhere taints the whole file)
  • mypy .

Disclaimer: I have not tried this, only took it from the top of my head from when I've tried to debug mypy cache issues previously..

@christianbundy
Copy link
Contributor

christianbundy commented Feb 5, 2023

I think you're right. I put together a minimal repro here: https://github.com/christianbundy/django-stubs-760

Calling ./repro.sh yields:

$ rm -rf .mypy_cache
$ mypy foo.py bar.py
Success: no issues found in 2 source files
$ echo 'x = 42' >> bar.py
$ mypy foo.py bar.py
Traceback (most recent call last):
  File "/opt/homebrew/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/opt/homebrew/lib/python3.10/site-packages/mypy/__main__.py", line 15, in console_entry
    main()
  File "mypy/main.py", line 95, in main
  File "mypy/main.py", line 174, in run_build
  File "mypy/build.py", line 193, in build
  File "mypy/build.py", line 276, in _build
  File "mypy/build.py", line 2903, in dispatch
  File "mypy/build.py", line 3284, in process_graph
  File "mypy/build.py", line 3365, in process_fresh_modules
  File "mypy/build.py", line 2112, in fix_cross_refs
  File "mypy/fixup.py", line 53, in fixup_module
  File "mypy/fixup.py", line 127, in visit_symbol_table
  File "mypy/nodes.py", line 1054, in accept
  File "mypy/fixup.py", line 179, in visit_var
  File "mypy/types.py", line 1283, in accept
  File "mypy/fixup.py", line 205, in visit_instance
  File "mypy/types.py", line 1283, in accept
  File "mypy/fixup.py", line 196, in visit_instance
  File "mypy/fixup.py", line 331, in lookup_fully_qualified_typeinfo
  File "mypy/lookup.py", line 49, in lookup_fully_qualified
AssertionError: Cannot find component 'WithAnnotations[django__contrib__auth__models__User]' for 'django_stubs_ext.WithAnnotations[django__contrib__auth__models__User]'

@christianbundy
Copy link
Contributor

Now, if one attempts to go with no_serialize=False and start writing to cache. It opens up a whole new can of worms -> #700

Is there more to it than this? I've edited the file locally and changed no_serialize=True to no_serialize=False, but I'm still able to reproduce the same error. I was paranoid that maybe I had multiple versions of django-stubs installed, so I ran it inside a Python 3.11 container with only django-stubs[compatible-mypy] installed and got the same error:

Traceback (most recent call last):
  File "/usr/local/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
             ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/__main__.py", line 15, in console_entry
    main()
  File "/usr/local/lib/python3.11/site-packages/mypy/main.py", line 95, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/main.py", line 174, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 193, in build
    result = _build(
             ^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 276, in _build
    graph = dispatch(sources, manager, stdout)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 2903, in dispatch
    process_graph(graph, manager)
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 3284, in process_graph
    process_fresh_modules(graph, prev_scc, manager)
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 3365, in process_fresh_modules
    graph[id].fix_cross_refs()
  File "/usr/local/lib/python3.11/site-packages/mypy/build.py", line 2112, in fix_cross_refs
    fixup_module(self.tree, self.manager.modules, self.options.use_fine_grained_cache)
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 53, in fixup_module
    node_fixer.visit_symbol_table(tree.names, tree.fullname)
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 127, in visit_symbol_table
    value.node.accept(self)
  File "/usr/local/lib/python3.11/site-packages/mypy/nodes.py", line 1054, in accept
    return visitor.visit_var(self)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 179, in visit_var
    v.type.accept(self.type_fixer)
  File "/usr/local/lib/python3.11/site-packages/mypy/types.py", line 1283, in accept
    return visitor.visit_instance(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 205, in visit_instance
    a.accept(self)
  File "/usr/local/lib/python3.11/site-packages/mypy/types.py", line 1283, in accept
    return visitor.visit_instance(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 196, in visit_instance
    inst.type = lookup_fully_qualified_typeinfo(
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/fixup.py", line 331, in lookup_fully_qualified_typeinfo
    stnode = lookup_fully_qualified(name, modules, raise_on_missing=not allow_missing)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/mypy/lookup.py", line 49, in lookup_fully_qualified
    assert key in names, f"Cannot find component {key!r} for {name!r}"
           ^^^^^^^^^^^^
AssertionError: Cannot find component 'WithAnnotations[django__contrib__auth__models__User]' for 'django_stubs_ext.WithAnnotations[django__contrib__auth__models__User]'

I don't see any reference to this type in .mypy_cache/3.11/django_stubs_ext/, am I looking in the wrong place?

@fjsj
Copy link

fjsj commented Mar 24, 2023

In case you don't have a code to reproduce this, I could reproduce in my project with a code like this:

    def export(queryset: QuerySet):
        queryset: FooQuerySet
        return queryset.annotate(...)

@christianbundy
Copy link
Contributor

Yep, full reproduction repository is linked above: #760 (comment)

@christianbundy
Copy link
Contributor

christianbundy commented May 24, 2023

Just wanted to quickly note two findings:

  1. If you need to work around this, you can set the variable type to a QuerySet and you won't have this problem. You will have errors if you try to access your annotations though.

  2. I was hopeful that I'd be able to circumvent this by requiring explicit annotations for all .annotate() usages, but unfortunately that doesn't solve the problem. You end up with errors like:

class F(TypedDict):
    foo: str

annotated: WithAnnotations[User, F] = User.objects.annotate(foo=Value("")).get()
AssertionError: Cannot find component "WithAnnotations[django__contrib__auth__models__User, TypedDict('foo" for "django_stubs_ext.WithAnnotations[django__contrib__auth__models__User, TypedDict('foo.F', {'foo': builtins.str})]"

@christianbundy
Copy link
Contributor

Would anyone be opposed to a plugin option to disable WithAnnotations? This bug has been around for 18+ months and disabling the cache has a significant performance costs, and in codebases where this feature is net negative it seems like it might be best to disable it until we have an opportunity to fix it.

@AustinScola
Copy link

Not sure I understand what that would accomplish? I think code that doesn't use WithAnnotations can still use the cache just fine?

@henribru
Copy link
Contributor

henribru commented Jun 7, 2023

Not sure I understand what that would accomplish? I think code that doesn't use WithAnnotations can still use the cache just fine?

You don't have to have WithAnnotations anywhere in your code base to run into this issue. The reproduction linked earlier doesn't. I assume it can be enough for the plugin to infer WithAnnotations

@AustinScola
Copy link

Ahh, wow. So this bug is worse than I thought. Is it just me or does it seem like it shouldn't be that hard to fix...

@AustinScola
Copy link

I would pay somebody $ to fix it

@flaeppe
Copy link
Member

flaeppe commented Jun 7, 2023

Would anyone be opposed to a plugin option to disable WithAnnotations? This bug has been around for 18+ months and disabling the cache has a significant performance costs, and in codebases where this feature is net negative it seems like it might be best to disable it until we have an opportunity to fix it.

I would welcome it, and also prefer if WithAnnotations was opt-in as default configuration. My reasoning being that there's insufficient upstream support for plugins to integrate .annotates level of varying attributes on an instance. Loosing cache is far worse than a couple of dynamic attributes, one can take multiple approaches to, manually, attach types to annotated attributes.

Quick investigation gave me the following 2 places to branch at in order to make it configurable:

1

elif method_name == "annotate":
info = self._get_typeinfo_or_none(class_fullname)
if info and info.has_base(fullnames.QUERYSET_CLASS_FULLNAME) or class_fullname in manager_classes:
return partial(querysets.extract_proper_type_queryset_annotate, django_context=self.django_context)

2

def get_type_analyze_hook(self, fullname: str) -> Optional[Callable[[AnalyzeTypeContext], MypyType]]:
if fullname in (
"typing.Annotated",
"typing_extensions.Annotated",
"django_stubs_ext.annotations.WithAnnotations",
):
return partial(handle_annotated_type, django_context=self.django_context)
else:
return None

@flaeppe
Copy link
Member

flaeppe commented Jun 7, 2023

A minor boilerplate to, quite cheaply, attach types to annotations (which I often use myself) is an approach that hides the annotated attribute behind a property, which instead controls the attribute during runtime.

class MyQuerySet(models.QuerySet["MyModel"]):
    def with_foo(self) -> MyQuerySet:
        return self.annotate(foo_value=Sum(...))

MyManager = models.Manager.from_queryset(MyQuerySet)

class MyModel(models.Model):
    ...

    objects = MyManager()

    @property
    def foo(self) -> int:
        assert hasattr(self, "foo_value"), "Call queryset with `.with_foo()`"
        return self.foo_value

x: int = MyModel.objects.with_foo().get().foo

As long as one covers the grounds touching .foo in your test suite I guess it should probably work out alright.

@panchock
Copy link

It's still happening:

  • mypy==1.3.1, django-stubs[compatible-mypy]==4.2.1
  • mypy==1.4.1, django-stubs[compatible-mypy]==4.2.3

Anyone found workaround for this issue without deleting .mypy_cache dir every run?

@moe-salek
Copy link

Any updates/solutions? It may not be the most prudent decision to consistently remove or disable the cache.

@pm-incyan
Copy link

I've just hit this same issue on current versions of all packages.

@asottile-sentry
Copy link

please stop bumping the thread -- if you're interested in updates click [subscribe] on the right. if you're also hitting this issue click :thumbsup: on the original post

a lot of us are subscribed to this thread to get updates and sending extra emails does not help it get prioritized or fixed!

@sobolevn
Copy link
Member

sobolevn commented Aug 5, 2024

#2319 introduces a (breaking!) fix to this issue. But, a correct one (at least it seems this way) :)

@AustinScola
Copy link

#2319 introduces a (breaking!) fix to this issue. But, a correct one (at least it seems this way) :)

Will there be a release with Django 4.2 support that has this fix?

@sobolevn
Copy link
Member

sobolevn commented Aug 8, 2024

@AustinScola nope, sorry. We don't have a capacity for backports and multiple versions support.

@flaeppe
Copy link
Member

flaeppe commented Aug 8, 2024

An additional note is that it's also depending on mypy>=1.11

@intgr
Copy link
Collaborator

intgr commented Aug 8, 2024

It's possible to use django-stubs 5.0.x with Django 4.2, but there are some caveats to be aware of. See this post:

@quimey
Copy link

quimey commented Sep 4, 2024

Is there an upcoming release in pypi with this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash "Internal error" crashes from mypy mypy-plugin Issues specific to mypy_django_plugin
Development

Successfully merging a pull request may close this issue.