-
Notifications
You must be signed in to change notification settings - Fork 117
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
QuerySet is inferred as list #165
Comments
Is this not a Manager getting inferred as a list? In any case, yes, I'm seeing this a lot:
And myriad others. Will try and take a look, see how this can be fixed. Also, on the same versions as above. |
The problem comes from the second line. In this example pylint-django uses transformations which try to teach it what type an object/attribute is and what other attributes there are. For this to be resolved effectively we need to teach pylint-django about all Manager methods that return a QuerySet. @carlio ^^^ not sure what I'm getting myself into with this proposition. Ideas ? |
@atodorov well pylint-django is already a huge shim and things like this exist: https://github.com/PyCQA/pylint-django/blob/master/pylint_django/augmentations/__init__.py#L37 and also https://github.com/PyCQA/pylint-django/blob/master/pylint_django/transforms/fields.py#L31 The whole thing is about second-guessing Django metaprogramming to fit into pylint's expectation of what things are. So what you're getting yourself into is: adding more special cases to lots of special cases because that's really all that can be done! |
Though another thought - originally the problem was that any import of django.models resulted in django trying to configure itself, which is why there are many workarounds. This was from 4 years ago though so that might be out-of-date. Perhaps a 'try: django.configure() except: <load some default config in the pylint-django lib' could work now? If the environment is not set up to load whatever config users have, then just fall back on some simple basics just to get django.conf.settings to be available and that'd solve lots of the problems of having to shim things together I would guess. Just an idea, I haven't tried this at all. |
✅🔨 Fix linting errors in tests Upgrade: * faker from 0.8.15 to 0.8.17 * sphinx-rtd-theme from 0.3.1 to 0.4.0 * sphinx from 1.7.4 to 1.7.6 * django-extensions from 2.0.7 to 2.1.0 * astroid from 1.6.4 to 1.6.5 * pylint from 1.9.1 to 1.9.2 * setuptools from 39.2.0 to 40.0.0 * tox from 3.0.0 to 3.1.2 Further upgrades to PyLint, Astroid, and pylint-django ignored: - pylint-dev/pylint-django#173 - pylint-dev/pylint-django#165
The problem seems to be caused by a transform in pylint-django that substitutes a Manager by something that returns a list for all the methods that return QuerySets: all(), filter(), etc. This doesn't allow for the same methods on the QuerySet itself, so it doesn't recognise chaining, e.g. MyModel.objects.filter(...).select_related(...).order_by(...) |
Can you retry with pylint-django 2.0 that was released today ? |
It works. Thank you. |
@matsui-goga the problem is that the latest pylint and django don't support 2.7 either so there's no point in pylint-django supporting it. Old versions of all those three still do though. |
… (no-member)" See: pylint-dev/pylint-django#165 Also updated documentation for enrollment metrics
If you define the
objects
attribute of a model asmodels.Manager()
, you get erroron
I'm using pylint 1.9.1, pylint-django 0.11.1.
The text was updated successfully, but these errors were encountered: