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

Bump Pylint to 2.17 #351

Closed
wants to merge 13 commits into from
Closed

Bump Pylint to 2.17 #351

wants to merge 13 commits into from

Conversation

bebound
Copy link
Contributor

@bebound bebound commented Feb 6, 2023

Bump pylint to support Python 3.11.

pylint==2.11.1 breaks vcrpy, as wrapt 1.13 does not support 3.11.
wrapt support 3.11 in 1.14 https://wrapt.readthedocs.io/en/latest/changes.html#version-1-14-0

├── pylint==2.11.1                                                                                  python code static checker
│   ├── astroid<2.9,>=2.8.0                                                                         An abstract syntax tree for Python with inference support.
│   │   ├── lazy-object-proxy>=1.4.0                                                                A fast and thorough lazy object proxy.
│   │   ├── setuptools>=20.0                                                                        Easily download, build, install, upgrade, and uninstall Python packages
│   │   └── wrapt<1.14,>=1.11

Related issue: Azure/azure-cli#24494

This also makes flake8 upgrade from 4.0.1 to 6.0.0, as explained in #351 (comment)

@@ -53,7 +53,7 @@ class CliGroupDirective(CliBaseDirective):
Field('docsource', label='Doc Source', has_arg=False,
names=('docsource', 'documentsource')),
Field('deprecated', label='Deprecated', has_arg=False,
names=('deprecated'))
names=('deprecated',))
Copy link
Contributor Author

@bebound bebound Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tox.ini Outdated
@@ -13,5 +13,5 @@ whitelist_externals =
commands=
python ./scripts/license_verify.py
python setup.py check -r -s
pylint azdev --rcfile=.pylintrc -r n
pylint azdev --rcfile=.pylintrc -r n --load-plugins=pylint.extensions.no_self_use
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add load-plugins to prevent azdev/operations/help/refdoc/common/directives.py:46:0: R0022: Useless option value for 'disable', 'no-self-use' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it make more sense if we remove # pylint: disable=no-self-use instead? Pylint apparently doesn't think no-self-use is important anymore: pylint-dev/pylint#5502

@bebound bebound mentioned this pull request Feb 6, 2023
4 tasks
@bebound bebound changed the title Bump pylint to 2.16.1 Bump Pylint to 2.16.1 Feb 6, 2023
return False

def add_target_and_index(self, name, sig, signode):
signode['ids'].append(name)

def get_index_text(self, modname, name): # pylint: disable=unused-argument, no-self-use
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise it raises azdev/operations/help/refdoc/common/directives.py:46:0: R0022: Useless option value for 'disable', 'no-self-use' was moved to an optional extension, see https://pylint.pycqa.org/en/latest/whatsnew/2/2.14/summary.html#removed-checkers. (useless-option-value)

@@ -8,7 +8,7 @@
import re
import sys

from distutils.version import LooseVersion # pylint:disable=import-error,no-name-in-module
from distutils.version import LooseVersion # pylint:disable=import-error,no-name-in-module,deprecated-module
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporally disable this warning.
I will create another PR to fix this like Azure/azure-cli#17667

# code flow is too complex, too many violations, to be removed in the future
C901,
# line break after binary operator effect on readability is subjective
W504
Copy link
Contributor Author

@bebound bebound Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix ValueError: Error code '#' supplied to 'ignore' option does not match '^[A-Z]{1,3}[0-9]{0,3}$'
Flake8 does not support inline comments for any of the keys, so comments should be put in newline.
Related issue: PyCQA/flake8#1750

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When pylint==2.11.1, it installs mccabe 0.6.1. flake8 also requires mccabe, and only 4.0.1 meet the version constraint.

  - pylint [required: ==2.11.1, installed: 2.11.1]
    - astroid [required: >=2.8.0,<2.9, installed: 2.8.6]
      - lazy-object-proxy [required: >=1.4.0, installed: 1.9.0]
      - setuptools [required: >=20.0, installed: 63.2.0]
      - wrapt [required: >=1.11,<1.14, installed: 1.13.3]
    - isort [required: >=4.2.5,<6, installed: 5.12.0]
    - mccabe [required: >=0.6,<0.7, installed: 0.6.1]
    - platformdirs [required: >=2.2.0, installed: 2.6.2]
    - toml [required: >=0.7.1, installed: 0.10.2]
 
- flake8 [required: Any, installed: 4.0.1]
    - mccabe [required: >=0.6.0,<0.7.0, installed: 0.6.1]
    - pycodestyle [required: >=2.8.0,<2.9.0, installed: 2.8.0]
    - pyflakes [required: >=2.4.0,<2.5.0, installed: 2.4.0]

After upgrading PyLint 2.16.1, mccabe becomes [required: >=0.6,<0.8, installed: 0.7.0], and flake8 becomes 6.0.0.

Copy link
Member

@jiasli jiasli Jul 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this issue still exist in pylint 2.17.4? https://pypi.org/project/pylint/

@bebound bebound force-pushed the update-pylint branch 2 times, most recently from 56a5863 to 7237f08 Compare February 6, 2023 08:34
@bebound bebound force-pushed the update-pylint branch 3 times, most recently from 887d2a5 to 2fd99af Compare February 7, 2023 06:51
@bebound bebound marked this pull request as ready for review February 8, 2023 09:07
@dciborow
Copy link
Contributor

dciborow commented Mar 10, 2023

@bebound , great work here!

I've used your branch as a base to upgrade pylint to 2.17.0, while also renaming some of the settings which changed after pylint 2.14 per pylint-dev/pylint#6931

Made a PR into your branch here - bebound#1

and a fresh PR to the repo here - #359

@bebound bebound changed the title Bump Pylint to 2.16.1 Bump Pylint to 2.17 Mar 13, 2023
@bebound
Copy link
Contributor Author

bebound commented Aug 10, 2023

Close as #359 is merged

@bebound bebound closed this Aug 10, 2023
@bebound bebound deleted the update-pylint branch July 12, 2024 08:57
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

Successfully merging this pull request may close these issues.

3 participants