Open asottile-sentry opened 1 year ago
partially reverting #1672 fixes this so I suspect the removal of objects
there isn't correct CC @flaeppe
--- a/mypy_django_plugin/transformers/models.py
+++ b/mypy_django_plugin/transformers/models.py
@@ -833,9 +833,9 @@ class MetaclassAdjustments(ModelClassInitializer):
):
del ctx.cls.info.names["MultipleObjectsReturned"]
- objects = ctx.cls.info.names.get("objects")
- if objects is not None and isinstance(objects.node, Var) and not objects.plugin_generated:
- del ctx.cls.info.names["objects"]
+ # objects = ctx.cls.info.names.get("objects")
+ # if objects is not None and isinstance(objects.node, Var) and not objects.plugin_generated:
+ # del ctx.cls.info.names["objects"]
return
I was looking at this a little earlier and I'm suspecting there's a configuration mismatch involved. I wasn't able to start up sentry though, but I admin I didn't try too hard either.
Anyways, I think all models within that file are test models but the django-stubs
plugin relies heavily (unfortunately) on the runtime. So unless I'm reading stuff wrong, I'm suspecting that the runtime that django-stubs
loads doesn't include the models specified in tests/sentry/db/models/fields/test_jsonfield.py
? (Essentially since their app isn't in INSTALLED_APPS
)
If they don't, the plugin will now delete objects
but never add it back.. So perhaps the plugin should be more careful before deleting the objects
attribute from what it sees as non-registered models..
I haven't verified this, but I think if one would set up an editable install of django-stubs
and place a strategic print
or so in here:
@cached_property
def model_modules(self) -> Dict[str, Dict[str, Type[Model]]]:
"""All modules that contain Django models."""
modules: Dict[str, Dict[str, Type[Model]]] = defaultdict(dict)
for concrete_model_cls in self.apps_registry.get_models():
modules[concrete_model_cls.__module__][concrete_model_cls.__name__] = concrete_model_cls
# collect abstract=True models
for model_cls in concrete_model_cls.mro()[1:]:
if issubclass(model_cls, Model) and hasattr(model_cls, "_meta") and model_cls._meta.abstract:
modules[model_cls.__module__][model_cls.__name__] = model_cls
+ print(modules)
return modules
You could see which models django-stubs
collects from the runtime.. I'm suspecting e.g. JSONFieldTestModel
isn't included..
Another way might be to add in e.g. tests.sentry.db
, if that's the app, to INSTALLED_APPS
, run mypy
again and see if any errors goes away.
Additionally I just want to point out that the real solution here is to have the plugin not add managers from the runtime model, but instead analyze the declared model(and its parents) and figure which manager attributes are implicitly added..
was removing objects
solving something specific? it seems to be breaking a lot given the pinned issue and this and I'm not sure it's worth it at this point if we can't get it correct even with runtime analysis
was removing
objects
solving something specific? [...]
Not more than an attempt of better alignment with the runtime..
We probably need to relax that deletion until analysis of models is more sophisticated and independent of runtime models, if that'll ever happen
You could see which models
django-stubs
collects from the runtime.. I'm suspecting e.g.JSONFieldTestModel
isn't included..
Thanks for this @flaeppe, this solved my issue! The model in question hadn't been exported properly and wasn't visible to the app config. Very oblique error message and root cause, would not have guessed the issue.
Confused on this issue. I've pretty much switched to ._default_manager in my projects. You can add this to the exceptions in your linter like Ruff. It's odd but it's better than the workarounds IMO.
Bug report
unfortunately having a really hard time tracking this down -- I've seen #1684 already however this is a concrete model and not an abstract one
What's wrong
I haven't been able to narrow this to a minimal case so I suspect it's something wrong with our setup -- here's at least a maximal reproduction:
output:
will try and narrow it down to a minimal case but I've failed to do that so far
How is that should be
I expect no errors in that file
System information
python
version: 3.8.16django
version: 3.2.20mypy
version: 1.5.1django-stubs
version: 4.2.4django-stubs-ext
version: 4.2.2