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

Remove pylint #2205

Merged
merged 3 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ ENV/
# snapshots
*.tar

# pylint config is left at dev's discretion, CI uses ruff/isort/black instead
.pylintrc
AlexanderDokuchaev marked this conversation as resolved.
Show resolved Hide resolved

# VSCode
.vscode/

Expand Down
52 changes: 0 additions & 52 deletions .pylintrc

This file was deleted.

9 changes: 6 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ If your testing code is more extensive than unit tests (in terms of test executi

Changes to NNCF Python code should conform to [Python Style Guide](./docs/styleguide/PyGuide.md)

Pylint is used throughout the project to ensure code cleanliness and quality.
A Pylint run is also done as part of the pre-commit scope - the pre-commit `pytest` scope will not be run if your code fails the Pylint checks.
The Pylint rules and exceptions for this repository are described in the standard [.pylintrc](./.pylintrc) format - make sure your local linter uses these.
Basic code style and static checks are enforced using a `pre-commit` Github action.
AlexanderDokuchaev marked this conversation as resolved.
Show resolved Hide resolved
The exact checks that are run are described in the corresponding [config file](./.pre-commit-config.yaml).
The checks can be run locally using `make pre-commit`.
Most of these checks do in-place fixes at the same time they are run using `make pre-commit`.
Static analysis is done via `ruff` which does not do in-place fixes by default - run `ruff --fix` locally to autofix errors for which it is possible to do so.
Developers may use other tools locally (such as `pylint`) as long as no tool-specific control code is introduced and the pre-commit checks pass.

## Binary files

Expand Down
32 changes: 4 additions & 28 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ endif
install-pre-commit:
pip install pre-commit==3.2.2

install-pylint:
pip install pylint==2.13.9
pip install pylintfileheader==0.3.2

###############################################################################
# ONNX backend
Expand All @@ -31,15 +28,12 @@ install-onnx-test:
pip install -r tests/cross_fw/examples/requirements.txt
pip install -r tests/onnx/benchmarking/requirements.txt

install-onnx-dev: install-onnx-test install-pre-commit install-pylint
install-onnx-dev: install-onnx-test install-pre-commit
pip install -r examples/post_training_quantization/onnx/mobilenet_v2/requirements.txt

test-onnx:
pytest ${COVERAGE_ARGS} tests/onnx $(DATA_ARG) --junitxml ${JUNITXML_PATH}

pylint-onnx:
pylint --rcfile .pylintrc \
$(shell python3 tools/collect_pylint_input_files_for_backend.py onnx)

test-install-onnx:
pytest tests/cross_fw/install -s \
Expand All @@ -60,7 +54,7 @@ install-openvino-test:
pip install -r tests/cross_fw/install/requirements.txt
pip install -r tests/cross_fw/examples/requirements.txt

install-openvino-dev: install-openvino-test install-pre-commit install-pylint
install-openvino-dev: install-openvino-test install-pre-commit
pip install -r examples/experimental/openvino/bert/requirements.txt
pip install -r examples/experimental/openvino/yolo_v5/requirements.txt
pip install -r examples/post_training_quantization/openvino/mobilenet_v2/requirements.txt
Expand All @@ -71,10 +65,6 @@ install-openvino-dev: install-openvino-test install-pre-commit install-pylint
test-openvino:
pytest ${COVERAGE_ARGS} tests/openvino $(DATA_ARG) --junitxml ${JUNITXML_PATH}

pylint-openvino:
pylint --rcfile .pylintrc \
$(shell python3 tools/collect_pylint_input_files_for_backend.py openvino)

test-install-openvino:
pytest tests/cross_fw/install -s \
--backend openvino \
Expand All @@ -95,18 +85,14 @@ install-tensorflow-test:
pip install -r tests/cross_fw/examples/requirements.txt
pip install -r examples/tensorflow/requirements.txt

install-tensorflow-dev: install-tensorflow-test install-pre-commit install-pylint
install-tensorflow-dev: install-tensorflow-test install-pre-commit
pip install -r examples/post_training_quantization/tensorflow/mobilenet_v2/requirements.txt

test-tensorflow:
pytest ${COVERAGE_ARGS} tests/tensorflow \
--junitxml ${JUNITXML_PATH} \
$(DATA_ARG)

pylint-tensorflow:
pylint --rcfile .pylintrc \
$(shell python3 tools/collect_pylint_input_files_for_backend.py tensorflow)

test-install-tensorflow:
pytest tests/cross_fw/install -s --backend tf --junitxml ${JUNITXML_PATH}

Expand All @@ -123,7 +109,7 @@ install-torch-test:
pip install -r tests/cross_fw/examples/requirements.txt
pip install -r examples/torch/requirements.txt --index-url https://download.pytorch.org/whl/cu118 --extra-index-url=https://pypi.org/simple

install-torch-dev: install-torch-test install-pre-commit install-pylint
install-torch-dev: install-torch-test install-pre-commit
pip install -r examples/post_training_quantization/torch/mobilenet_v2/requirements.txt
pip install -r examples/post_training_quantization/torch/ssd300_vgg16/requirements.txt

Expand All @@ -136,12 +122,6 @@ test-torch-nightly:
test-torch-weekly:
pytest ${COVERAGE_ARGS} tests/torch -m weekly --junitxml ${JUNITXML_PATH} $(DATA_ARG) ${WEEKLY_MODELS_ARG}

COMMON_PYFILES := $(shell python3 tools/collect_pylint_input_files_for_backend.py common)
pylint-torch:
pylint --rcfile .pylintrc \
$(COMMON_PYFILES) \
$(shell python3 tools/collect_pylint_input_files_for_backend.py torch)

test-install-torch-cpu:
pytest tests/cross_fw/install -s \
--backend torch \
Expand All @@ -167,10 +147,6 @@ install-common-test:
pip install -r tests/cross_fw/install/requirements.txt
pip install -r tests/cross_fw/examples/requirements.txt

pylint-common:
pylint --rcfile .pylintrc \
$(COMMON_PYFILES)

test-common:
pytest ${COVERAGE_ARGS} tests/common $(DATA_ARG) --junitxml ${JUNITXML_PATH}

Expand Down
7 changes: 3 additions & 4 deletions docs/api/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def __init__(self):
self.canonical_name_vs_fqn: Dict[str, str] = {}


# pylint: disable=too-many-branches
def collect_api_entities() -> APIInfo:
"""
Collects the fully qualified names of symbols in NNCF package that contain a special attribute (set via
Expand All @@ -73,21 +72,21 @@ def collect_api_entities() -> APIInfo:
for _, modname, _ in pkgutil.walk_packages(path=nncf.__path__, prefix=nncf.__name__ + ".", onerror=lambda x: None):
try:
modules[modname] = importlib.import_module(modname)
except Exception as e: # pylint: disable=broad-except
except Exception as e:
skipped_modules[modname] = str(e)

from nncf.common.utils.api_marker import api

canonical_imports_seen = set()
# pylint: disable=too-many-nested-blocks

for modname, module in modules.items():
print(f"{modname}")
for obj_name, obj in inspect.getmembers(module):
objects_module = getattr(obj, "__module__", None)
if objects_module == modname:
if inspect.isclass(obj) or inspect.isfunction(obj):
if hasattr(obj, api.API_MARKER_ATTR):
marked_object_name = obj._nncf_api_marker # pylint: disable=protected-access
marked_object_name = obj._nncf_api_marker
# Check the actual name of the originally marked object
# so that the classes derived from base API classes don't
# all automatically end up in API
Expand Down
78 changes: 28 additions & 50 deletions docs/styleguide/PyGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
- [1 Introduction](#s1-introduction)
- [2 Automating Code Formatting](#s2-auto-code-formatting)
- [3 Python Language Rules](#s3-python-language-rules)
- [3.1 PyLint](#s3.1-pylint)
- [3.2 3rd party packages](#s3.2-3rd-party-packages)
- [3.3 Global variables](#s3.3-global-variables)
- [3.4 Nested/Local/Inner Classes and Functions](#s3.4-nested)
- [3.5 Default Iterators and Operators](#s3.5-default-iterators-and-operators)
- [3.6 Type Annotated Code](#s3.6-type-annotated-code)
- [3.7 Files and Sockets](#s3.7-files-and-sockets)
- [3.8 Abstract Classes](#s3.8-abstract-classes)
- [3.1 3rd party packages](#s3.1-3rd-party-packages)
- [3.2 Global variables](#s3.2-global-variables)
- [3.3 Nested/Local/Inner Classes and Functions](#s3.3-nested)
- [3.4 Default Iterators and Operators](#s3.4-default-iterators-and-operators)
- [3.5 Type Annotated Code](#s3.5-type-annotated-code)
- [3.6 Files and Sockets](#s3.6-files-and-sockets)
- [3.7 Abstract Classes](#s3.7-abstract-classes)
- [4 Python Style Rules](#s4-python-style-rules)
- [4.1 Line length](#s4.1-line-length)
- [4.2 Comments and Docstrings](#s4.2-comments-and-docstrings)
Expand Down Expand Up @@ -102,39 +101,19 @@ arr2 = [

## 3 Python Language Rules

<a id="s3.1-pylint"></a>
<a id="31-pylint"></a>
<a id="pylint"></a>

### 3.1 PyLint
AlexanderDokuchaev marked this conversation as resolved.
Show resolved Hide resolved

Run [pylint](https://github.com/PyCQA/pylint) over your code using this [pylintrc](../../.pylintrc).

- Every warning reported by [pylint](https://github.com/PyCQA/pylint) must be resolved in one of the following way:
- *Preferred solution*: Change the code to fix the warning.
- *Exception*: Suppress the warning if they are inappropriate so that other issues are not hidden.
To suppress warnings you can set a line-level comment

```python
dict = "something awful" # Bad Idea... pylint: disable=redefined-builtin
```

or update [pylintrc](../../.pylintrc) if applicable for the whole project. If the reason for the suppression
is not clear from the symbolic name, add an explanation.

<a id="s3.2-3rd-party-packages"></a>
<a id="32-3rd-party-packages"></a>
<a id="s3.1-3rd-party-packages"></a>
<a id="31-3rd-party-packages"></a>
<a id="3rd-party-packages"></a>

### 3.2 3rd party packages
### 3.1 3rd party packages

Do not add new third-party dependencies unless absolutely necessary. All things being equal, give preference to built-in packages.

<a id="s3.3-global-variables"></a>
<a id="33-global-variables"></a>
<a id="s3.2-global-variables"></a>
<a id="32-global-variables"></a>
<a id="global-variables"></a>

### 3.3 Global variables
### 3.2 Global variables

Avoid global variables.

Expand All @@ -143,11 +122,11 @@ Avoid global variables.
- If needed, globals should be declared at the module level and made internal to the module by prepending an `_` to the
name. External access must be done through public module-level functions.

<a id="s3.4-nested"></a>
<a id="34-nested"></a>
<a id="s3.3-nested"></a>
<a id="33-nested"></a>
<a id="nested-classes-functions"></a>

### 3.4 Nested/Local/Inner Classes and Functions
### 3.3 Nested/Local/Inner Classes and Functions

No need to overuse nested local functions or classes and inner classes.

Expand Down Expand Up @@ -191,11 +170,11 @@ No need to overuse nested local functions or classes and inner classes.
return m/3
```

<a id="s3.5-default-iterators-and-operators"></a>
<a id="35-default-iterators-and-operators"></a>
<a id="s3.4-default-iterators-and-operators"></a>
<a id="34-default-iterators-and-operators"></a>
<a id="default-iterators-operators"></a>

### 3.5 Default Iterators and Operators
### 3.4 Default Iterators and Operators

Use default iterators and operators for types that support them, like lists,
dictionaries, and files. The built-in types define iterator methods, too. Prefer
Expand All @@ -219,11 +198,11 @@ for line in afile.readlines(): ...
for k, v in dict.iteritems(): ...
```

<a id="s3.6-type-annotated-code"></a>
<a id="36-type-annotated-code"></a>
<a id="s3.5-type-annotated-code"></a>
<a id="35-type-annotated-code"></a>
<a id="type-annotated-code"></a>

### 3.6 Type Annotated Code
### 3.5 Type Annotated Code

Code should be annotated with type hints according to
[PEP-484](https://www.python.org/dev/peps/pep-0484/), and type-check the code at
Expand All @@ -233,11 +212,11 @@ build time with a type checking tool like [mypy](http://www.mypy-lang.org/).
def func(a: int) -> List[int]:
```

<a id="s3.7-files-and-sockets"></a>
<a id="37-files-and-sockets"></a>
<a id="s3.6-files-and-sockets"></a>
<a id="36-files-and-sockets"></a>
<a id="files-and-sockets"></a>

### 3.7 Files and Sockets
### 3.6 Files and Sockets

Explicitly close files and sockets when done with them.

Expand All @@ -247,11 +226,11 @@ with open("hello.txt") as hello_file:
print(line)
```

<a id="s3.8-abstract-classes"></a>
<a id="38-abstract-classes"></a>
<a id="s3.7-abstract-classes"></a>
<a id="37-abstract-classes"></a>
<a id="abstract-classes"></a>

### 3.8 Abstract Classes
### 3.7 Abstract Classes

When defining abstract classes, the following template should be used:

Expand Down Expand Up @@ -312,7 +291,6 @@ Explicit exceptions to the 120 character limit:
- URLs, pathnames, or long flags in comments.
- Long string module level constants not containing whitespace that would be
inconvenient to split across lines such as URLs or pathnames.
- Pylint disable comments. (e.g.: `# pylint: disable=invalid-name`)

<a id="s4.2-comments-and-docstrings"></a>
<a id="42-comments-and-docstrings"></a>
Expand Down
2 changes: 0 additions & 2 deletions examples/common/sample_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,11 @@ def __call__(self, parser, namespace, values, option_string=None):
return self._action(parser, namespace, values, option_string)


# pylint:disable=protected-access
class CustomArgumentGroup(argparse._ArgumentGroup):
def _add_action(self, action):
super()._add_action(ActionWrapper(action))


# pylint:disable=protected-access
class CustomActionContainer(argparse._ActionsContainer):
def add_argument_group(self, *args, **kwargs):
group = CustomArgumentGroup(self, *args, **kwargs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def main(argv):
start_worker(main_worker, config)


# pylint:disable=too-many-branches,too-many-statements
def main_worker(current_gpu, config: SampleConfig):
configure_device(current_gpu, config)
config.mlflow = SafeMLFLow(config)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ def main(argv):
start_worker(main_worker, config)


# pylint:disable=too-many-branches,too-many-statements
def main_worker(current_gpu, config: SampleConfig):
configure_device(current_gpu, config)
config.mlflow = SafeMLFLow(config)
Expand Down
Loading