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

unused argument schema_editor in django data migrations #267

Closed
lokesh1729 opened this issue Jun 8, 2020 · 5 comments
Closed

unused argument schema_editor in django data migrations #267

lokesh1729 opened this issue Jun 8, 2020 · 5 comments

Comments

@lokesh1729
Copy link

lokesh1729 commented Jun 8, 2020

in django when writing data migrations, the function definition would like this

def update_something(apps, schema_editor):
    # do something

here, apps and schema_editor will be passed by django when applying migration. There are cases where either of them will not be used. In such cases pylint-django should not complain unused-argument error.

EXPECTED : it is expected that to have a exception for this case

It happened many times that folks in our team removes schema_editor when pylint complains with unused-argument and that result into errors.

here's pip freeze

appdirs==1.4.4
appnope==0.1.0
argh==0.26.2
aspy.yaml==1.3.0
astroid==2.4.1
attrs==19.3.0
Babel==2.8.0
backcall==0.1.0
better-exceptions==0.2.2
billiard==3.6.3.0
bleach==3.1.5
celery==4.4.1
certifi==2020.4.5.1
cfgv==2.0.1
chardet==3.0.4
coreapi==2.3.3
coreschema==0.0.4
coverage==5.1
decorator==4.4.2
defusedxml==0.6.0
distlib==0.3.0
Django==2.1.5
django-extensions==2.1.6
django-redis-cache==2.1.0
django-rest-swagger==2.1.2
djangorestframework==3.9.3
entrypoints==0.3
Faker==2.0.3
filelock==3.0.12
flower==0.9.3
gevent==1.4.0
greenlet==0.4.15
gunicorn==20.0.4
identify==1.4.15
idna==2.8
importlib-metadata==1.6.0
importlib-resources==1.5.0
ipdb==0.13.2
ipykernel==5.2.1
ipython==7.9.0
ipython-genutils==0.2.0
ipywidgets==7.5.1
isort==4.3.21
itypes==1.2.0
jedi==0.17.0
Jinja2==2.10.3
jsonschema==3.2.0
jupyter==1.0.0
jupyter-client==6.1.3
jupyter-console==6.1.0
jupyter-core==4.6.3
kombu==4.6.8
lazy-object-proxy==1.4.3
lxml==4.5.0
MarkupSafe==1.1.1
mccabe==0.6.1
mistune==0.8.4
more-itertools==8.2.0
msgpack==1.0.0
nbconvert==5.6.1
nbformat==5.0.6
neovim==0.3.1
nodeenv==1.3.5
notebook==6.0.3
openapi-codec==1.3.2
packaging==20.3
pandocfilters==1.4.2
parso==0.7.0
pathlib2==2.3.5
pathtools==0.1.2
pexpect==4.8.0
pickleshare==0.7.5
Pillow==6.2.0
pluggy==0.13.1
pre-commit==1.21.0
prometheus-client==0.7.1
prompt-toolkit==2.0.10
psycopg2-binary==2.8.4
ptyprocess==0.6.0
py==1.8.1
Pygments==2.6.1
pylint==2.5.3
pylint-django==2.0.15
pylint-plugin-utils==0.6
pynvim==0.4.1
pyparsing==2.4.7
PyPDF2==1.26.0
pyrsistent==0.16.0
pytest==5.4.2
pytest-cov==2.6.1
pytest-django==3.4.5
pytest-ordering==0.6
python-dateutil==2.8.1
python-pptx==0.6.18
pytz==2020.1
PyYAML==5.3.1
pyzmq==19.0.1
qtconsole==4.7.3
QtPy==1.9.0
redis==3.4.1
reportlab==3.5.28
requests==2.22.0
Send2Trash==1.5.0
simplejson==3.17.0
six==1.14.0
terminado==0.8.3
testpath==0.4.4
text-unidecode==1.3
toml==0.10.0
tornado==5.1.1
traitlets==4.3.3
typed-ast==1.4.1
uritemplate==3.0.1
urllib3==1.25.9
vine==1.3.0
virtualenv==20.0.20
watchdog==0.9.0
wcwidth==0.1.9
webencodings==0.5.1
widgetsnbextension==3.5.1
wrapt==1.12.1
XlsxWriter==1.2.8
zipp==1.2.0
@carlio
Copy link
Collaborator

carlio commented Jun 8, 2020

So if you have a function like that:

def update_something(apps, schema_editor):
    Person = apps.get_model('yourappname', 'Person')
    # don't use schema_editor

There are several possibilities here:

def update_something(apps, _schema_editor):
   ...

Pylint does not warn about arguments starting with an underscore. You can still see that it is a schema_editor but pylint won't issue the warning. I think this is cleaner than calling it _ which you could also do.

Secondly you could explicitly ignore the warning on the line:

def update_something(apps, schema_editor):  # pylint: disable=unused-argument
  ...

In this way you can fix your specific problem without needing pylint-django to have a new release.

@lokesh1729
Copy link
Author

@carlio : thanks for looking into this.... i know about the second option to disable a specific line, but adding that to every migration would be painful, but the first option makes sense. I didn't know about that... hence closing the issue

@carlio
Copy link
Collaborator

carlio commented Jun 8, 2020

@lokesh1729 no worries, check out this section of the documentation: http://pylint.pycqa.org/en/latest/technical_reference/features.html#variables-checker-options

By default _something and ignored_something and unused_something will not generate errors, but you can configure that value in your .pylintrc to whatever you want.

@atodorov
Copy link
Contributor

Reopening b/c pyint_django should be smart enough to ignore these unused arguments in a similar way we do for view methods.

@atodorov atodorov reopened this Jun 10, 2020
@atodorov
Copy link
Contributor

Fixed via #273

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants