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

Add type annotation update pre commit #122

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
45 changes: 38 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,40 @@
# Installation as pre-commit hook:
# $ pip install pre-commit
#
# This is the configuration file of the pre-commit framework for this repository:
# https://pypi.org/project/pre-commit
#
# pre-commit runs in the GitHub Workflow of this project on each push and PR.
# Additionally, you can run it locally for faster fixing of issues using
# $ pip3 install pre-commit -r requirements-dev.txt
#
# On the initial run, pre-commit downloads its checkers, subsequent runs are fast:
#
# $ pre-commit run # automatically checks which checks are needed.
# $ pre-commit run -a # runs all fixes and checks, even when not needed
#
# When this works, you can enable it as the pre-commit hook for this git clone:
# $ pre-commit install
# $ pre-commit install --hook-type pre-push
#
# Global installation as a git-template (for new clones):
# $ git config --global init.templateDir ~/.git-template
# $ pre-commit init-templatedir ~/.git-template
# See: https://pre-commit.com/#automatically-enabling-pre-commit-on-repositories
# All hooks: https://pre-commit.com/hooks.html
#
# You can skip checks if you commit very often you don't want them to run, e.g:
# export SKIP=mypy,pylint;git commit -m "quick save" (or for --amend)
#
# For more customisation, see https://pre-commit.com/#temporarily-disabling-hooks
# and https://pre-commit.com/#confining-hooks-to-run-at-certain-stages (e.g push)
#
# After this, the pre-commit fixes and checks run when you commit an update.
#
# Install pre-commit as a global pre-commit hook(for all repos with a config):
# $ git config --global init.templateDir ~/.git-template
# $ pre-commit init-templatedir ~/.git-template
#
# Detailed Usage information for all possible situations:
# https://github.com/pre-commit/pre-commit.com/blob/main/sections/advanced.md
# Cheat sheet on the config file keywords:
# https://github.com/dexpota/cheatsheets/blob/master/pre-commit
exclude: "^tests/data"
fail_fast: true
default_stages: [commit]
Expand Down Expand Up @@ -40,29 +69,31 @@ repos:
pass_filenames: true
- id: shellcheck
- id: mdformat-check
exclude: README-Unicode.md
- repo: https://github.com/pycqa/pylint
rev: v2.17.4
hooks:
- id: pylint
args:
[
-sn, # Don't display the score
--rcfile=pylintrc,
--load-plugins=pylint.extensions.eq_without_hash,
--ignore-imports=yes,
"--disable=duplicate-code,superfluous-parens,line-too-long",
"--disable=duplicate-code,line-too-long",
]
log_file: ".git/pre-commit-pylint.log"
additional_dependencies:
- pyfakefs
- six
- mock
- pandas
- pytest_forked
- toml
- repo: local
hooks:
- id: run-pyre
name: run-pyre
entry: python run-pyre.py
entry: python pyre_runner.py
types: [python]
language: python
log_file: ".git/pre-commit-pyre.log"
Expand Down
5 changes: 3 additions & 2 deletions pylintrc → .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# Python code to execute, usually for sys.path manipulation such as
# pygtk.require().
# For absolute sys.path, use this for getting our stubs path dynamically:
init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()) + '/stubs')"
init-hook="import os,sys;sys.path.append(os.path.dirname('stubs'))"

# Profiled execution.
profile=no
Expand Down Expand Up @@ -50,7 +50,7 @@ load-plugins=
#
disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
bad-option-value, # Skip complaining about suppressions for older option values
unrecognized-option, # Skip complaining on pylintrc options only in pylint2/pylint3
unrecognized-option, # Skip complaining on .pylintrc options only in pylint2/pylint3
unknown-option-value, # Skip complaining about checkers only in pylint2/pylint3
useless-object-inheritance, # "object" is not obsolete for supporting Python2
super-with-arguments, # super() with arguments is a Python3-only feature
Expand All @@ -61,6 +61,7 @@ disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
consider-using-dict-items,
consider-using-enumerate,
consider-using-ternary,
duplicate-code,
len-as-condition,
no-else-return,
raise-missing-from,
Expand Down
10 changes: 5 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ Installation of additional python versions for testing different versions:
- If `deadsnakes/ppa` does not work, e.g. for Python 3.6, `conda` or `pyenv` can be used.
For instructions, see https://realpython.com/intro-to-pyenv:
```yaml
sudo apt install -y build-essential xz-utils zlib1g-dev
sudo apt install -y build-essential xz-utils zlib1g-dev \
lib{bz2,ffi,lzma,readline,ssl,sqlite3}-dev
curl https://pyenv.run | bash # add displayed commands to .bashrc
pyenv install 3.{6,8,11} && pyenv local 3.{6,8,11} # builds them
~/.pyenv/bin/pyenv install 3.{6,8,11} && ~/.pyenv/bin/pyenv local 3.{6,8,11} # builds them
```
- For testing on newer Ubuntu which has `python2-dev`, but not `pip2`, install `pip2` this way:
```yml
Expand All @@ -37,7 +37,7 @@ You may want to install `pytype` in your user environment to run it directly wit
# On Python != 3.8, pytype can't import xml.dom.minidom, use 3.8:
python3.8 -m pip install pytype
python -m pip install tabulate
./run-pytype.py
./pytype_runner.py
```

## Installation of dependencies using `pip`
Expand Down Expand Up @@ -79,8 +79,8 @@ Using pip-tools, you can extract the requirements and extras from `pyptoject.tom
```bash
PYTHON=python3.10
EXTRAS=.,test,mypy,pyre,pytype,tox
PFLAGS="--no-warn-conflicts --resolver=backtracking"
$PYTHON -m pip install pip-tools
PFLAGS="--no-warn-conflicts"
$PYTHON -m pip install pip-tools==7.3.0
$PYTHON -m piptools compile --extra=$EXTRAS -o - pyproject.toml |
$PYTHON -m pip install -r /dev/stdin $PFLAGS
```
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ $(MY_OUTPUT_DIR)/python-libs.inc: $(MY_OUTPUT_DIR)/.dirstamp Makefile

.PHONY: pylint
pylint:
run-pylint.sh $(PYTHON_LIBS_SOURCES)
pylint_runner.sh $(PYTHON_LIBS_SOURCES)

.PHONY: sources
sources: $(RPM_SRPMSDIR)/$(PYTHON_LIB_SRPM)
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ The Continuous Integration Tests feature:
- Automatic Upload of the combined coverage to CodeCov (from the GitHub Workflow)
- Checking of the combined coverage against the diff to master: Fails if changes are not covered!
- Pylint report in the GitHub Action Summary page, with Warning and Error annotatios, even in the code review.
- Check that changes don't generate pylint warnings (if warning classes which are enabled in pylintrc)
- Check that changes don't generate pylint warnings (if warning classes which are enabled in .pylintrc)
- Static analysis using `mypy`, `pyre` and `pytype`

This enforces that any change (besides whitespace):

- has code coverage and
- does not introduce a `pylint` warning which is not disabled in `pylintrc`
- does not introduce a `pylint` warning which is not disabled in `.pylintrc`
- does not introduce a type of static analysis warning which is currently suppressed.

## Status Summary
Expand All @@ -54,7 +54,7 @@ open a recent workflow run the latest and scroll down until you see the tables!
- `pytest.ini`: The defaults used by `pytest` unless overruled by command line options
- `.github/workflows/main.yml`: Configuration of the GitHub CI matrix jobs and coverage upload
- `.github/act-serial.yaml`: Configuration for the jobs run by the local GitHub actions runner `act`
- `pylintrc`: Configuration file of `Pylint`
- `.pylintrc`: Configuration file of `Pylint`

## Installation and Setup of a development environment

Expand All @@ -71,8 +71,8 @@ For the installation of the general development dependencies, visit [INSTALL.md]
issues by Python3.6
- Test with `python2.7 -m pytest`
- Run `mypy` (without any arguments - The configuration is in `pyproject.toml`)
- Run `./run-pytype.py`
- Run `./run-pyre.py`
- Run `./pytype_runner.py`
- Run `./pyre_runner.py`
- Run `tox -e py36-lint` and fix any `Pylint` warnings
- Run `tox -e py310-covcombine-check` and fix any missing diff-coverage.
- Run `tox` for the full CI test suite
Expand Down
15 changes: 7 additions & 8 deletions run-pylint.py → pylint_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@
from io import StringIO
from typing import List, TextIO

import pandas as pd # type: ignore[import]
from pylint.lint import Run # type: ignore[import]
from pylint.reporters import JSONReporter # type: ignore[import]
from toml import load

import pandas as pd # type: ignore[import]


def del_dict_keys(r, *args):
for arg in args:
Expand Down Expand Up @@ -108,11 +107,11 @@ def cleanup_results_dict(r, sym):
# https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/eq-without-hash.html
#
pylint_options: List[str] = [
"--load-plugins", "pylint.extensions.eq_without_hash",
"--load-plugins", "pylint.extensions.eq_without_hash"
]

def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):

def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
pylint_overview = []
pylint_results = []
pylint_paths = []
Expand All @@ -125,13 +124,13 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
smells_total = 0
for path in pylint_paths:
filename = path.rsplit("/", maxsplit=1)[-1]
if filename in ["__init__.py", "pylintrc"]:
if filename in ["__init__.py", ".pylintrc"]:
continue
reporter_buffer = StringIO()
results = Run(
[path] + pylint_options,
reporter=JSONReporter(reporter_buffer),
do_exit=False,
exit=False,
)
score = results.linter.stats.global_note
file_results = json.loads(reporter_buffer.getvalue())
Expand Down Expand Up @@ -169,11 +168,11 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):

message_ids[sym] = "" # Populate a dict of the pylint message symbols seen in this file
# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message
endline = r['endLine']
endline = r["endLine"]
end = f",endLine={endline}" if endline and endline != lineno else ""
print(
f"::{cls} file={path},line={lineno}{end},"
f"title=pylint {msg_id}: {sym}::{msg}"
f"title=pylint {msg_id}: {sym}::{msg}",
)
r["path"] = f"[{linktext}]({branch_url}/{path}#L{lineno})"
cleanup_results_dict(r, sym)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ build-backend = "setuptools.build_meta"
packages = ["xcp", "xcp.net", "xcp.net.ifrename"]

[tool.black]
line-length = 100
line-length = 96

[tool.mypy]
pretty = true
Expand Down
File renamed without changes.
5 changes: 3 additions & 2 deletions run-pytype.py → pytype_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ def run_pytype_and_parse_annotations(xfail_files: List[str], branch_url: str):
print("No errors in", xfail_file)
return err_code or len(results), results


def to_markdown(me, fp, returncode, results, branch_url):
mylink = f"[{me}]({branch_url}/{me}.py)"
pytype_link = "[pytype](https://google.github.io/pytype)"
Expand All @@ -162,7 +163,7 @@ def setup_and_run_pytype_action(scriptname: str):
branch = os.environ.get("GITHUB_HEAD_REF", None) or os.environ.get("GITHUB_REF_NAME", None)
filelink_baseurl = f"{server_url}/{repository}/blob/{branch}"
retcode, results = run_pytype_and_parse_annotations(xfail_files, filelink_baseurl)
# Write the panda dable to a markdown output file:
# Write the panda dable to a markdown output file:
summary_file = os.environ.get("GITHUB_STEP_SUMMARY", None)
if summary_file:
with open(summary_file, "w", encoding="utf-8") as fp:
Expand All @@ -174,4 +175,4 @@ def setup_and_run_pytype_action(scriptname: str):
if __name__ == "__main__":
script_basename = os.path.basename(__file__).split(".")[0]
basicConfig(format=script_basename + ": %(message)s", level=INFO)
sys.exit(setup_and_run_pytype_action(scriptname = script_basename))
sys.exit(setup_and_run_pytype_action(scriptname=script_basename))
3 changes: 2 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
"""

# pyre-ignore-all-errors[21]
import pytest # pyre does not find the module when run by tox -e py311-pyre
import warnings

import pytest # pyre does not find the module when run by tox -e py311-pyre

@pytest.fixture(autouse=True)
def set_warnings():
"""
Expand Down
1 change: 1 addition & 0 deletions tests/test_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def test_file_accessor(fs):
# type:(FakeFilesystem) -> None
"""Test FileAccessor.writeFile(), .openAddress and .access using pyfakefs"""
accessor = xcp.accessor.createAccessor("file://repo/", False)
assert isinstance(accessor, xcp.accessor.FileAccessor)
check_binary_read(accessor, "/repo", fs)
check_binary_write(accessor, "/repo", fs)

Expand Down
5 changes: 3 additions & 2 deletions tests/test_cpiofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ def test_cpiofile_modes(fs):
assert exc_info.type == TypeError
if sys.version_info < (3, 0):
# Test Python2 pattern from host-upgrade-plugin:/etc/xapi.d/plugins/prepare_host_upgrade.py
import StringIO
# Import the Python2-only StringIO.StringIO module, imported as Py2StringIO:
from StringIO import Py2StringIO # pylint: disable=import-outside-toplevel

stringio = StringIO.StringIO()
stringio = Py2StringIO()
archive = CpioFile.open(fileobj=stringio, mode="w|gz") # type: ignore[arg-type]
archive.hardlinks = False
archive.close()
Expand Down
12 changes: 8 additions & 4 deletions tests/test_httpaccessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@
import base64
import sys
from contextlib import contextmanager
from typing import IO, Generator, Tuple
from io import BufferedReader, TextIOWrapper
from typing import Generator, Tuple

from six.moves import urllib # pyright: ignore

from xcp.accessor import HTTPAccessor, createAccessor
from xcp.accessor import createAccessor, HTTPAccessor

from .httpserver_testcase import ErrorHandler, HTTPServerTestCase, Response

HTTPAccessorGenerator = Generator[Tuple[HTTPAccessor, IO[bytes]], None, None]
HTTPAccessorGenerator = Generator[Tuple[HTTPAccessor, BufferedReader], None, None]

UTF8TEXT_LITERAL = "✋Hello accessor from the 🗺, download and verify me! ✅"

Expand All @@ -22,6 +23,7 @@ class HTTPAccessorTestCase(HTTPServerTestCase):
def test_404(self):
self.httpserver.expect_request("/404").respond_with_data("", status=404)
httpaccessor = createAccessor(self.httpserver.url_for("/"), True)
assert isinstance(httpaccessor, HTTPAccessor)
self.assertFalse(httpaccessor.access("404"))
self.httpserver.check_assertions()
self.assertEqual(httpaccessor.lastError, 404)
Expand All @@ -33,7 +35,7 @@ def http_get_request_data(self, url, read_file, error_handler):
self.serve_file(self.document_root, read_file, error_handler)

httpaccessor = createAccessor(url, True)
self.assertEqual(type(httpaccessor), HTTPAccessor)
assert isinstance(httpaccessor, HTTPAccessor)

with open(self.document_root + read_file, "rb") as ref:
yield httpaccessor, ref
Expand Down Expand Up @@ -98,5 +100,7 @@ def test_httpaccessor_open_text(self):
"""Get text containing UTF-8 and compare the returned decoded string contents"""
self.httpserver.expect_request("/textfile").respond_with_data(UTF8TEXT_LITERAL)
accessor = createAccessor(self.httpserver.url_for("/"), True)
assert isinstance(accessor, HTTPAccessor)
with accessor.openText("textfile") as textfile:
assert isinstance(textfile, TextIOWrapper)
assert textfile.read() == UTF8TEXT_LITERAL
3 changes: 1 addition & 2 deletions tests/test_ifrename_dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ def setUp(self):
openLog(self.logbuf, logging.NOTSET)

def tearDown(self):

closeLogs()
self.logbuf.close()
closeLogs()

def test_null(self):

Expand Down
Loading