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

Type from settings not correctly loaded in a model

Open lfrodrigues opened this issue 4 years ago • 18 comments

Bug report

I have this in settings:

BANK_ENTITIES = [('a', 'A Bank'), ('b', 'B Bank'),]

and this in my model:

entity = models.CharField(max_length=20, choices=settings.BANK_ENTITIES, blank=True)

What's wrong

mypy fails with:

models.py:247: error: Argument "choices" to "CharField" has incompatible type "tuple"; expected "Optional[Iterable[Union[Tuple[Any, Any], Tuple[str, Iterable[Tuple[Any, Any]]]]]]"

If I place a reveal in settings and another in models.py I get

settingsbase.py:90: note: Revealed type is 'builtins.list[Tuple[builtins.str, builtins.str]]'
models.py:214: note: Revealed type is 'builtins.list'

How is that should be

mypy should be able to load the full type from settings, not only that it's a list

System information

  • OS: Ubuntu 18.04
  • python version: 3.6
  • django version: 2.2
  • mypy version: 0.761
  • django-stubs version: 1.4.0

lfrodrigues avatar Jan 31 '20 13:01 lfrodrigues

Can you please post the result of reveal_type(BANK_ENTITIES) inside your settings and reveal_type(settings.BANK_ENTITIES) inside your models?

sobolevn avatar Jan 31 '20 14:01 sobolevn

I posted it in the initial report:

settingsbase.py:90: note: Revealed type is 'builtins.list[Tuple[builtins.str, builtins.str]]'
models.py:214: note: Revealed type is 'builtins.list'

lfrodrigues avatar Jan 31 '20 14:01 lfrodrigues

Oh, sorry, I have missed it. Yes, looks like a bug! Thanks for the report.

sobolevn avatar Jan 31 '20 14:01 sobolevn

Getting the same issue. Is there a plan to fix it @sobolevn ?

settings:

class POINTS:
    a = 1
    b = 20
    c = 5

models.py

from django.conf import settings
reveal_type(settings.POINTS)   #  Revealed type is 'builtins.type'
settings.POINTS.a  # [mypy error] [E] "type" has no attribute "a"

EDIT: Actually, my situation is explained in https://mypy.readthedocs.io/en/stable/metaclasses.html, and I was not defining a metaclass. Sorry for the disturb.

AdrienLemaire avatar Jun 16 '20 05:06 AdrienLemaire

@lfrodrigues could you verify if this bug still occurs? I wasn't able to reproduce it at all. I guess that someone fixed this in the meantime, or maybe your mypy.ini file wasn't properly configured.

kszmigiel avatar Jun 20 '20 21:06 kszmigiel

I just tested it again. If I just have a simple settings.py file like the one created when django starts a project there is no problem.

The problem is that my settings import different sub modules for settings depending on the country that was configured.

Here's a demo repository: https://github.com/lfrodrigues/testdjangotypes

from django.conf import settings

def view():
    a = settings.MY_SETTINGS['item']['subitem']

~/tmp/demo_error/src$ mypy app1/
app1/views.py:9: error: Value of type "_VT" is not indexable

lfrodrigues avatar Jun 20 '20 23:06 lfrodrigues

@lfrodrigues Thank you for the tips, I'll try to fix this as soon as I have enough time.

kszmigiel avatar Jun 21 '20 10:06 kszmigiel

I think I have a related issue

# settings.py
SOME_DICT = {
    'a': 'whatever',
    'b': 'whatever',
    'c': 'whatever',
}
# myproject/mymodule.py
from django.conf import settings

whatever = settings.SOME_DICT["b"]

and mypy gives the following error:

myproject/mymodule.py:3: error: Invalid index type "str" for "dict"; expected type "_KT"

The _KT TypeVar appears to be defined in mypy typeshed in relation to Mapping container, i.e. for dicts

Adding an annotation to the settings like this does not help:

# settings.py
from typing import Dict

SOME_DICT: Dict[str, str] = {
    'a': 'whatever',
    'b': 'whatever',
    'c': 'whatever',
}

The problem is that my settings import different sub modules for settings depending on the country that was configured.

we also have similar situation with conditional import of sub-module settings, maybe is relevant

(my example code above is simplified so might not reproduce the issue by itself)

I am able to make mypy happy with the following workaround:

# myproject/mymodule.py
from typing import Dict
from django.conf import settings

_some_dict: Dict[str, str] = settings.SOME_DICT

whatever = _some_dict["b"]

(and no annotation in settings.py)

anentropic avatar Sep 16 '20 15:09 anentropic

this looks the same https://github.com/typeddjango/django-stubs/issues/352

anentropic avatar Sep 22 '20 11:09 anentropic

I am experiencing the same bug with django-stubs 1.8.0 and django-stubs-ext 0.2.0. Defining dictionaries in a settings file and than importing them via django.conf.settings loses the type annotation, e.g.

# in settings.py
mydict = {"a": ["b", "c"]}
reveal_type(mydict) -> builtins.dict[builtins.str*, typing.Sequence*[builtins.str]]

becomes

# in other.py
from django.conf import settings
reveal_type(settings.mydict) -> builtings.dict

BeWe11 avatar Apr 15 '21 08:04 BeWe11

@kszmigiel, are you still intending to fix this or should it be unassigned?

RJPercival avatar May 27 '21 08:05 RJPercival

@RJPercival feel free to take this over! 👍

sobolevn avatar May 27 '21 10:05 sobolevn

Hello, I have the same issue, I tried to understand why it happens. In this function, plugin cant extract type from sym variable, because it's dict variable? after that, the plugin tries to understand what`s type, based on the variable value. https://github.com/typeddjango/django-stubs/blob/9a95b983981173d86b55c3ede97f637d94cfde9e/mypy_django_plugin/transformers/settings.py#L23-L49 In the case with dict value, the plugin gets type from builtins module. https://github.com/python/mypy/blob/master/mypy/typeshed/stdlib/builtins.pyi#L788

class dict(MutableMapping[_KT, _VT], Generic[_KT, _VT]):

I think, the plugin needs to check if its dict and return dict type from mypy module, like

Dict[Any, Any]

If it's the good solution I will make PR.

bogdan-zs avatar May 30 '21 16:05 bogdan-zs

Any chance to move this issue forward? Configuration keeps producing quite a lot of warnings and it is a bit difficult to work around it.

GergelyKalmar avatar Aug 10 '21 18:08 GergelyKalmar

Here’s a complete minimal reproducible test case in ~20 lines. Note that bad.py and good.py are identical, but they show different types from reveal_type. All that distinguishes them is that bad happens to be imported from my_app.models.

bad.py:4: note: Revealed type is "builtins.list"
good.py:4: note: Revealed type is "builtins.list[builtins.int]"

bad.py

from django.conf import settings

def test() -> None:
    reveal_type(settings.MY_LIST)

good.py

from django.conf import settings

def test() -> None:
    reveal_type(settings.MY_LIST)

my_app/__init__.py

# empty

my_app/models.py

from django.db import models
import bad

class MyUser(models.Model):
    pass

my_settings.py

from other import other

AUTH_USER_MODEL = "my_app.MyUser"
INSTALLED_APPS = ["my_app"]
MY_LIST = [1]

other.py

from django_stubs_ext import ValuesQuerySet

other = ()

pyproject.toml

[tool.django-stubs]
django_settings_module = "my_settings"

[tool.mypy]
plugins = ["mypy_django_plugin.main"]

Reproduction:

$ pip install Django \
    'git+https://github.com/typeddjango/django-stubs.git@6f9afd3c#egg=django-stubs[compatible-mypy]' \
    'git+https://github.com/typeddjango/django-stubs.git@6f9afd3c#subdirectory=django_stubs_ext'

$ pip freeze
asgiref==3.5.2
Django==4.1.1
django-stubs @ git+https://github.com/typeddjango/django-stubs.git@6f9afd3cc9202a3d1147f78e53ef31ced2f03452
django-stubs-ext==0.6.0
mypy==0.971
mypy-extensions==0.4.3
sqlparse==0.4.2
tomli==2.0.1
types-pytz==2022.2.1.0
types-PyYAML==6.0.11
typing_extensions==4.3.0

$ mypy .
bad.py:4: note: Revealed type is "builtins.list"
good.py:4: note: Revealed type is "builtins.list[builtins.int]"
Success: no issues found in 6 source files

Notes:

  • The Git version of django-stubs seems to be necessary for this test case. A git bisect finds that the problem reproduces with 8b5509b2ab88c0bca82a1163bafb1ad5b65e4bd5 and not with its parent. Of course, we’ve seen reports from well before then, so that’s probably a fluke.
  • I’m not sure what role other.py plays, but it seems to be necessary for reproduction.

andersk avatar Sep 20 '22 02:09 andersk

In my test case, during the first run of get_type_of_settings_attribute (for bad.py), module.names is

SymbolTable(
  AUTH_USER_MODEL : Gdef/Var (my_settings.AUTH_USER_MODEL) : builtins.str
  INSTALLED_APPS : Gdef/Var (my_settings.INSTALLED_APPS)
  MY_LIST : Gdef/Var (my_settings.MY_LIST)
  other : Gdef/Var (other.other) : Tuple[])

so sym.type is None and it falls back to inspecting the runtime value of value.__class__, resulting in builtins.list. But during the second run (for good.py), module.names is now

SymbolTable(
  AUTH_USER_MODEL : Gdef/Var (my_settings.AUTH_USER_MODEL) : builtins.str
  INSTALLED_APPS : Gdef/Var (my_settings.INSTALLED_APPS) : builtins.list[builtins.str]
  MY_LIST : Gdef/Var (my_settings.MY_LIST) : builtins.list[builtins.int]
  other : Gdef/Var (other.other) : Tuple[])

so sym.type is correctly builtins.list[builtins.int].

It seems to me that falling back to the runtime type isn’t how a static type checker should ever behave. I don’t know much about the mypy API, but if the static type hasn’t been inferred yet, surely there must be a way to trigger mypy to finish inferring the type and/or delay until it has done so.

andersk avatar Sep 20 '22 03:09 andersk

The reproduction seems plausible, but I'm not yet able to reproduce it in the pytest environment. I guess this is very specific to how the modules are set up and how mypy is invoked. There is a defer_node method available on the type checker, but I will need to verify if it suits this use case.

PIG208 avatar Sep 21 '22 15:09 PIG208

@PIG208 Here’s my example as a failing pytest case: #1159.

andersk avatar Sep 22 '22 00:09 andersk

Deferring type analysis for an attribute seems to be tricky, as I can't find direct support for that. The only relevant plugin API for deferring is only supported for get_dynamic_class_hook and get_base_class_hook. Attempted to fix this by returning PlaceholderType or UnboundType, but that didn't work.

PIG208 avatar Sep 22 '22 22:09 PIG208

Actually, this might have more to do with the get_additional_deps hook. Any modules accessing settings should depend on the settings module.

PIG208 avatar Sep 23 '22 20:09 PIG208

Tracking the debugging process in #1162. We need to come up with a proper fix for this issue.

PIG208 avatar Sep 24 '22 01:09 PIG208