django-stubs icon indicating copy to clipboard operation
django-stubs copied to clipboard

WithAnnotations Crash in 1.9.0

Open lfrodrigues opened this issue 4 years ago • 29 comments

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 avatar Nov 28 '21 21:11 lfrodrigues

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 avatar Feb 25 '22 17:02 AustinScola

Maybe related to https://github.com/typeddjango/django-stubs/pull/725?

AustinScola avatar Feb 25 '22 21:02 AustinScola

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

AustinScola avatar May 25 '22 15:05 AustinScola

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

AdrienLemaire avatar Jun 29 '22 11:06 AdrienLemaire

After removing the .mypy_cache directory it worked again

Pixel-Jack avatar Aug 04 '22 15:08 Pixel-Jack

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.

intgr avatar Aug 19 '22 13:08 intgr

Still having this issue in 1.13.0.

ashkop avatar Nov 07 '22 11:11 ashkop

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

tony avatar Nov 11 '22 14:11 tony

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?

christianbundy avatar Jan 23 '23 17:01 christianbundy

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

henribru avatar Jan 23 '23 18:01 henribru

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.

intgr avatar Jan 24 '23 08:01 intgr

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:

https://github.com/typeddjango/django-stubs/blob/9874789bea016a567cf8372cb0e695d9615a5324/mypy_django_plugin/transformers/models.py#L677-L683

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:

  • https://github.com/typeddjango/django-stubs/issues/760#issue-1065441644
  • https://github.com/typeddjango/django-stubs/pull/725#issuecomment-1137445039

flaeppe avatar Jan 24 '23 22:01 flaeppe

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

flaeppe avatar Jan 24 '23 22:01 flaeppe

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 avatar Feb 05 '23 19:02 christianbundy

Now, if one attempts to go with no_serialize=False and start writing to cache. It opens up a whole new can of worms -> https://github.com/typeddjango/django-stubs/issues/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?

christianbundy avatar Feb 06 '23 05:02 christianbundy

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

fjsj avatar Mar 24 '23 19:03 fjsj

Yep, full reproduction repository is linked above: https://github.com/typeddjango/django-stubs/issues/760#issuecomment-1418245576

christianbundy avatar Mar 24 '23 19:03 christianbundy

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 avatar May 24 '23 18:05 christianbundy

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.

christianbundy avatar Jun 07 '23 15:06 christianbundy

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

AustinScola avatar Jun 07 '23 15:06 AustinScola

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

henribru avatar Jun 07 '23 15:06 henribru

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 avatar Jun 07 '23 16:06 AustinScola

I would pay somebody $ to fix it

AustinScola avatar Jun 07 '23 16:06 AustinScola

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 https://github.com/typeddjango/django-stubs/blob/c75ada374e64bca4271b67793bd3a67b0350985a/mypy_django_plugin/main.py#L224-L227

2 https://github.com/typeddjango/django-stubs/blob/c75ada374e64bca4271b67793bd3a67b0350985a/mypy_django_plugin/main.py#L307-L315

flaeppe avatar Jun 07 '23 17:06 flaeppe

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.

flaeppe avatar Jun 07 '23 18:06 flaeppe

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?

panchock avatar Aug 18 '23 16:08 panchock

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

moe-salek avatar Feb 03 '24 11:02 moe-salek

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

pm-incyan avatar Mar 11 '24 15:03 pm-incyan

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!

asottile-sentry avatar Mar 11 '24 15:03 asottile-sentry