DEVOPS-1440 Add Precommit #195
Conversation
bio-boris
commented
Apr 24, 2024
bio-boris
commented
- This adds precommit hooks
- This PR also runs black and flake8 against the code and checks it in
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return web.FileResponse( | ||
| path.full_path, headers={"content-type": "application/octet-stream"} | ||
| ) | ||
| return web.FileResponse(path.full_path, headers={"content-type": "application/octet-stream"}) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
.pre-commit-config.yaml
Outdated
| - repo: https://github.com/pycqa/flake8 | ||
| rev: 7.0.0 | ||
| hooks: | ||
| - id: flake8 | ||
| args: | ||
| - --ignore=E203,W503 | ||
| - --max-line-length=120 | ||
| - --config=.flake8 | ||
| files: '\.py$' | ||
| additional_dependencies: [ flake8 ] |
There was a problem hiding this comment.
Ruff replaces flake8 - no need to have both
| @@ -34,21 +34,23 @@ def bootstrap_config(): | |||
| def assert_exception_correct(got: Exception, expected: Exception): | |||
| err = "".join(traceback.TracebackException.from_exception(got).format()) | |||
| assert got.args == expected.args, err | |||
| assert type(got) == type(expected) | |||
| assert type(got) == type(expected) # noqa: E721 | |||
There was a problem hiding this comment.
I presume this rule wants you to use isinstance instead of type? Better to remove the # noqa and either fix it in this PR or in an upcoming one since it's a simple fix.
There was a problem hiding this comment.
I believe I had discussions with @MrCreosote about how it needed to be this way rather than isinstance, but maybe I am mis-remembering. Can you chime in here @MrCreosote ?
There was a problem hiding this comment.
Because you want to assert against the exact exception type, not the type's inheritance hierarchy
There was a problem hiding this comment.
Oh -- in that case I'm sure that there are ways around this.
There was a problem hiding this comment.
What do I need to do next?
There was a problem hiding this comment.
For now, remove # noqa, it can be fixed in an upcoming PR (I am happy to fix it once you have finished your PRs).
There was a problem hiding this comment.
As we discussed in slack, we are keeping this one # noqa: E721 and changing the rest of them to be isinstance
There was a problem hiding this comment.
Changed the rest of the isinstance
| E("Invalid order_and_display entry for datatype t at index 1 - " | ||
| + "the entry is not a list")) | ||
| {"t": {"order_and_display": [["foo", "bar"], "baz"], "data": []}}, | ||
| E("Invalid order_and_display entry for datatype t at index 1 - " + "the entry is not a list"), |
| "data": [], | ||
| } | ||
| }, | ||
| E("Invalid order_and_display entry for datatype t at index 2 - " + "expected 2 item list"), |
| "data": [], | ||
| } | ||
| }, | ||
| E("Invalid order_and_display entry for datatype t at index 0 - " + "expected 2 item list"), |
| @@ -275,8 +387,7 @@ def test_xsv_parse_fail_missing_column_header_entries(temp_dir: Path): | |||
| def test_xsv_parse_fail_missing_data(temp_dir: Path): | |||
| err = "No non-header data in file" | |||
| lines = [ | |||
| "Data type: foo; Columns: 3; Version: 1\n" | |||
| "head1, head2, head3\n", | |||
| "Data type: foo; Columns: 3; Version: 1\n" "head1, head2, head3\n", | |||
There was a problem hiding this comment.
can these two strings be converted into a single string?
| @@ -285,40 +396,35 @@ def test_xsv_parse_fail_missing_data(temp_dir: Path): | |||
| def test_xsv_parse_fail_unequal_rows(temp_dir: Path): | |||
| err = "Incorrect number of items in line 3, expected 3, got 2" | |||
| lines = [ | |||
| "Data type: foo; Columns: 3; Version: 1\n" | |||
| "head1, head2, head3\n", | |||
| "Data type: foo; Columns: 3; Version: 1\n" "head1, head2, head3\n", | |||
There was a problem hiding this comment.
same here and below
|
This is the Ruff config that I've been using for recent projects -- although you may want to add it in one of the upcoming PRs if you're doing one focussed on dev tools. I just have all the available modules listed and comment out the ones that aren't appropriate. It's easier than looking up the docs every time! |
pyproject.toml
Outdated
| select = ["E4", "E7", "E9", "F"] | ||
| ignore = [] |
There was a problem hiding this comment.
this only checks a very small selection of errors. Let's start with the "core" error checks and we can add more as required.
select = [
# core
"F", # Pyflakes
"E", # pycodestyle errors
"W", # pycodestyle warnings
"C90", # mccabe +
"I", # isort
"N", # pep8-naming
# "D", # pydocstyle - disabled for now
"UP", # pyupgrade
]
a few ignores to add:
# E203: whitespace before ‘,’, ‘;’, or ‘:’
# E501: line length
# W503: line break after binary operator
ignore = [
"E203",
"E501",
"S101",
]
|
