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

chore: add overview of code quality tools; format code with black (DEV-2224) #391

Merged
merged 36 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2eaeac6
add black, remove isort, set line length = 120
jnussbaum May 31, 2023
fb68c26
Merge branch 'main' into wip/dev-2224-black
jnussbaum May 31, 2023
27151c6
start
jnussbaum May 31, 2023
2d6c764
second run
jnussbaum May 31, 2023
e0a6886
third run
jnussbaum May 31, 2023
51728f1
format some test files
jnussbaum Jun 1, 2023
a90b432
format rest of tests
jnussbaum Jun 1, 2023
1842ff8
reformat models
jnussbaum Jun 1, 2023
55db11a
second run
jnussbaum Jun 1, 2023
78d3007
update import_scripts
jnussbaum Jun 1, 2023
c7a7566
update submodule
jnussbaum Jun 1, 2023
3e552d6
fix unittests
jnussbaum Jun 1, 2023
ee4531a
resolve too long lines in src
jnussbaum Jun 1, 2023
9659eb2
resolve pylint errors
jnussbaum Jun 1, 2023
823c8da
update submodule
jnussbaum Jun 1, 2023
4165d1a
Merge branch 'main' into wip/dev-2224-black
jnussbaum Jun 1, 2023
82ee812
fix test
jnussbaum Jun 1, 2023
ae5ac57
move black to dev dependencies
jnussbaum Jun 1, 2023
b8ebc52
edit
jnussbaum Jun 1, 2023
5b461af
reformat readme
jnussbaum Jun 1, 2023
e266ca0
edit
jnussbaum Jun 2, 2023
68f76a3
edit
jnussbaum Jun 2, 2023
1dcecf0
remove unused pyright: ignore comments
jnussbaum Jun 2, 2023
436cc30
edit
jnussbaum Jun 2, 2023
e2ac6b5
reformat
jnussbaum Jun 2, 2023
f60db0f
edit
jnussbaum Jun 2, 2023
a5f2bc0
edit
jnussbaum Jun 5, 2023
ebd5af7
Merge branch 'main' into wip/dev-2224-black
jnussbaum Jun 5, 2023
e7f394b
split into subpages
jnussbaum Jun 5, 2023
a9e8653
tidy up
jnussbaum Jun 5, 2023
ac38fb0
edit
jnussbaum Jun 6, 2023
ab0eb0b
edit
jnussbaum Jun 6, 2023
1869689
edit
jnussbaum Jun 6, 2023
0359b69
fix link
jnussbaum Jun 6, 2023
54f1a9e
Merge branch 'main' into wip/dev-2224-black
jnussbaum Jun 6, 2023
cb70d58
blacken merge commit
jnussbaum Jun 6, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .editorconfig
@@ -1,6 +1,11 @@
# https://EditorConfig.org

# Unix-style newlines with a newline ending every file
[*]
end_of_line = lf
insert_final_newline = true

# 4 space indentation
[*.py]
indent_style = space
indent_size = 4
4 changes: 2 additions & 2 deletions .github/workflows/tests-on-push.yml
Expand Up @@ -59,8 +59,8 @@ jobs:
- name: Linting with mypy
run: poetry run mypy .

- name: Linting with isort
run: poetry run isort --check --diff --settings-path pyproject.toml .
- name: Linting with black
run: poetry run black --check .


# Install programs for local processing (fast xmlupload)
Expand Down
2 changes: 1 addition & 1 deletion .markdownlint.yml
@@ -1,6 +1,6 @@
---

# Config file for https://github.com/DavidAnson/markdownlint / https://github.com/igorshubovych/markdownlint-cli
# Config file for https://github.com/igorshubovych/markdownlint-cli

# MD006/ul-start-left - Consider starting bulleted lists at the beginning of the line
MD006: false
Expand Down
91 changes: 58 additions & 33 deletions README.md
@@ -1,57 +1,66 @@
[![PyPI version](https://badge.fury.io/py/dsp-tools.svg)](https://badge.fury.io/py/dsp-tools)
[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)

# DSP-TOOLS - DaSCH Service Platform Tools

DSP-TOOLS is a command line tool that helps you to interact with the DaSCH Service Platform API. This document is
intended for developers who want to work with the code of DSP-TOOLS.
DSP-TOOLS is a command line tool that helps you to interact with the DaSCH Service Platform API.
This document is intended for developers who want to work with the code of DSP-TOOLS.

| <center>Hint</center> |
|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| This technical document was written as a guide for developers. For the end user documentation, please consult [https://docs.dasch.swiss](https://docs.dasch.swiss/latest/DSP-TOOLS). |

This README contains basic information for getting started. More details can be found in the
This README contains basic information for getting started.
More details can be found in the
[developers documentation](https://docs.dasch.swiss/latest/DSP-TOOLS/developers/index/).



## Using poetry for dependency management

Curious what poetry is and why we use it? Check out the respective section in the
Curious what poetry is and why we use it?
Check out the respective section in the
[developers documentation](https://docs.dasch.swiss/latest/DSP-TOOLS/developers/packaging/).

If you want to work on the code of DSP-TOOLS, you first have to do the following:

- install poetry with `curl -sSL https://install.python-poetry.org | python3 -` (for Windows, see
[https://python-poetry.org/docs/](https://python-poetry.org/docs/))
- install poetry with `curl -sSL https://install.python-poetry.org | python3 -`
(for Windows, see [https://python-poetry.org/docs/](https://python-poetry.org/docs/))
- install the exec plugin with `poetry self add poetry-exec-plugin`
- execute `poetry install`, which will:
- create a virtual environment (if there isn't already one)
- install all dependencies (dev and non-dev) from `poetry.lock`.
If `poetry.lock` doesn't exist, it installs the dependencies from `pyproject.toml`, and creates a new `poetry.lock`.
If `poetry.lock` doesn't exist,
it installs the dependencies from `pyproject.toml`,
and creates a new `poetry.lock`.
- make an editable installation of DSP-TOOLS inside the virtual environment

There are two files defining the dependencies:

- `pyproject.toml` lists the direct dependencies, ordered in two sections:
- `[tool.poetry.dependencies]` lists the dependencies used to run the software.
- `[tool.poetry.group.dev.dependencies]` lists the dependencies used for developing and testing.
- `poetry.lock` enables deterministic installations, by exactly pinning the versions of all (sub-)dependencies.
- `poetry.lock` enables deterministic installations,
by exactly pinning the versions of all (sub-)dependencies.
This is done automatically, you must not edit `poetry.lock`.

If you want to install a new package, install it with `poetry add package`. This
If you want to install a new package,
install it with `poetry add package`. This

- installs the package (incl. sub-dependencies) in your virtual environment
- adds the package to the section `[tool.poetry.dependencies]` of `pyproject.toml`
- adds the pinned versions of the package and all sub-dependencies to `poetry.lock`

If a package is only needed for development, please install it with `poetry add package --group dev`,
If a package is only needed for development,
please install it with `poetry add package --group dev`,
so it will be added to the `[tool.poetry.group.dev.dependencies]` section of `pyproject.toml`.

For security reasons, the maintainer regularly executes `poetry add dependency@latest` for every dependency.
This updates the dependency constraint in `pyproject.toml` to the latest version,
updates the dependency to that version,
and updates `poetry.lock` accordingly.
The resulting changes are then committed in a version bumping PR.
For security reasons, the dependencies should be kept up to date.
GitHub's dependabot is configured to automatically create a version bumping PR
if there is an update for a dependency.
Version bumping PRs can also be created manually:
Execute `poetry add dependency@latest` for every dependency,
and create a PR from the resulting changes.

All developers working with the DSP-TOOLS repository should regularly execute `poetry self update` to update poetry,
and `poetry install` to update the dependencies from `poetry.lock`.
Expand Down Expand Up @@ -84,12 +93,14 @@ Find more information in the

## Publishing/distribution

Publishing is automated with GitHub Actions and should _not_ be done manually. Please follow the
[Pull Request Guidelines](https://docs.dasch.swiss/latest/developers/dsp/contribution/#pull-request-guidelines). If done
correctly, when merging a pull request into `main`, the `release-please` action will create or update a release
PR. This PR will follow semantic versioning and update the change log. Once all desired features are
merged, the release can be executed by merging this release pull request into `main`. This will trigger actions that
create a release on GitHub and on PyPI.
Publishing is automated with GitHub Actions and should _not_ be done manually.
Please follow the [Pull Request Guidelines](https://docs.dasch.swiss/latest/developers/dsp/contribution/#pull-request-guidelines).
If done correctly, when merging a pull request into `main`,
the `release-please` action will create or update a release PR.
This PR will follow semantic versioning and update the change log.
Once all desired features are merged,
the release can be executed by merging this release pull request into `main`.
This will trigger actions that create a release on GitHub and on PyPI.

Please ensure you have only one pull request per feature.

Expand All @@ -111,19 +122,20 @@ Or only the tests inside a singe file: `pytest test/unittests/test_excel2xml.py`
Or even a certain method: `pytest test/unittests/test_excel2xml.py::TestExcel2xml::test_make_boolean_prop`

This is useful in combination with the debugging mode of your IDE,
if you want to examine why
a single line of code in a test method fails.
if you want to examine why a single line of code in a test method fails.



## Code style

When contributing to the project, please make sure you use the same code style rules as we do. We use
When contributing to the project,
please make sure you use the same code style rules as we do.
We use

- [pylint](https://pypi.org/project/pylint/) (configured in `pyproject.toml`)
- [isort](https://pypi.org/project/isort/) (configured in `pyproject.toml`)
- [markdownlint](https://github.com/igorshubovych/markdownlint-cli) (configured in `.markdownlint.yml`)
- [black](https://pypi.org/project/black/) (configured in `pyproject.toml`)
- [mypy](https://pypi.org/project/mypy/) (configured in `pyproject.toml`)
- [markdownlint](https://github.com/DavidAnson/markdownlint) (configured in `.markdownlint.json`)
- [pylint](https://pypi.org/project/pylint/) (configured in `pyproject.toml`)

These 4 linters are integrated in the GitHub CI pipeline,
so that every pull request is checked for code style violations.
Expand All @@ -134,21 +146,34 @@ To locally check your code for style violations, follow the instructions dependi

In VSCode,

- pylint can be installed as extension (`ms-python.pylint`), and be configured in the VSCode settings.
- isort can be installed as extension (`ms-python.isort`), and be configured in the VSCode settings.
- mypy can be installed as extension (`matangover.mypy`), and be configured in the VSCode settings.
- (This extension is different from Microsoft's Python extension's mypy functionality,
which only lints each file separately, leading to incomplete type checking.)
- markdownlint can be installed as extension (`davidanson.vscode-markdownlint`), and be configured in the VSCode settings.
- markdownlint can be installed as extension (`davidanson.vscode-markdownlint`),
and be configured in the VSCode settings.
- black can be set as formatter in the `ms-python.python` extension.
To do so, set `"python.formatting.provider": "black"` in the VSCode `settings.json`.
Alternatively, `ms-python.black-formatter` can be installed as extension.
- mypy can be installed as extension (`matangover.mypy`),
and be configured in the VSCode settings.
- This extension is different from the mypy functionality
of Microsoft's Python extension `ms-python.python`,
which only lints each file separately,
leading to incomplete type checking.
- pylint can be installed as extension (`ms-python.pylint`),
and be configured in the VSCode settings.

If configured correctly, you will see style violations in the "Problems" tab.

Make sure to set the docstring format to "google-notypes" in the VSCode settings:
VS Code > Settings > Auto Docstring: Docstring Format > google-notypes.


### PyCharm

In PyCharm, mypy is available as [plugin](https://plugins.jetbrains.com/plugin/11086-mypy),
and many style checks can be enabled in Settings > Editor > Inspections > Python.

Make sure to set the docstring format to "Google notypes" in the PyCharm settings:
PyCharm > Settings > Tools > Python Integrated Tools > Docstring format: Google notypes


## Contributing to the documentation

Expand Down
28 changes: 28 additions & 0 deletions docs/developers/code-quality-tools/code-quality-tools.md
@@ -0,0 +1,28 @@
[![PyPI version](https://badge.fury.io/py/dsp-tools.svg)](https://badge.fury.io/py/dsp-tools)

# Code quality tools

There is a variety of tools that help to keep the code quality high.

DSP-TOOLS uses the ones listed on this page.

The decision to use this set of tools is based on the information in the following pages.


| Task | Tool | Configuration |
| --------------------------------------------------------------- | ------------------------------------------------------------------ | ------------------------ |
| [General formatting](./general-formatting.md) | [EditorConfig](https://EditorConfig.org/) | `.editorconfig` |
| | [markdownlint](https://github.com/igorshubovych/markdownlint-cli/) | `.markdownlint.yml` |
| [Python formatting](./python-formatting.md) | [Black](https://pypi.org/project/black/) | `pyproject.toml` |
| [Python docstring formatting](./python-docstring-formatting.md) | [pydocstyle](https://pypi.org/project/pydocstyle/) * | |
| [Python type checking](./python-type-checking.md) | [Mypy](https://pypi.org/project/mypy/) | `pyproject.toml` |
| [Python linting](./python-linting.md) | [Ruff](https://pypi.org/project/ruff/) * | |
| | [Pylint](https://pypi.org/project/pylint/) ** | `pyproject.toml` |
| [Security checks](./security.md) | [Dependabot](https://docs.github.com/en/code-security/dependabot/) | `.github/dependabot.yml` |
| | [CodeQL](https://codeql.github.com/) | GitHub settings |
| | [Gitleaks](https://gitleaks.io/) * | `.gitleaks.toml` |
| | [Bandit](https://pypi.org/project/bandit/) * | `pyproject.toml` |

(*) coming soon

(**) will perhaps become redundant, as Ruff matures
14 changes: 14 additions & 0 deletions docs/developers/code-quality-tools/general-formatting.md
@@ -0,0 +1,14 @@
[![PyPI version](https://badge.fury.io/py/dsp-tools.svg)](https://badge.fury.io/py/dsp-tools)

# General formatting

## [EditorConfig](https://EditorConfig.org)

Language-independent, cross-editor settings for indentation, line endings, etc.

## [markdownlint](https://github.com/igorshubovych/markdownlint-cli)

A CLI for David Anson's markdownlint, a static analysis tool with a library of rules.
The flexibility of the markdown syntax is both a benefit and a drawback.
Many styles are possible, so formatting can be inconsistent.
Some constructs don't work well in all parsers and should be avoided.
74 changes: 74 additions & 0 deletions docs/developers/code-quality-tools/python-docstring-formatting.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this should exist in our documentation. It makes sense to note somewhere, which style we follow; but this seems way out of scope of what we should have in our documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see your point. For me personally, it was important to make all these notes, and to store them somewhere where they can be accessed, and reevaluated. When the ecosystem will have changed in some years, and we don't have these notes, then it will be more difficult to assess if our choice of tools is still okay.

In my view, these notes make our choice more transparent.

And I would never have put these notes into the user part of the documentation. If they belong somewhere, then here in the developers/ADR section.

Writing this, the idea comes to my mind to move them to my personal github.io page. This would allow me to keep them stored somewhere, and to reference them, without cluttering the dsp-tools docs. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't recommend mixing private and work stuff too much... but that's up to you. However if it's on your personal page, you shouldn't expect other team members to search for it there.
To me, the most intuitive place for this kind of internal note-keeping would be Notion, as it is our "internal Wiki".

But at the end of the day, it really doesn't matter too much; and you can also just leave it in the docs.

@@ -0,0 +1,74 @@
[![PyPI version](https://badge.fury.io/py/dsp-tools.svg)](https://badge.fury.io/py/dsp-tools)

# Python Docstring formatting

## Docstring flavors

Python uses docstrings to document code.
A docstring is a string that is the first statement in a package, module, class or function.
Python docsrings are written in the
[reStructuredText](https://docutils.sourceforge.io/rst.html) syntax (abbreviated as RST or reST).

There are at least 4 flavors of docstrings,
each following the [PEP 257](http://www.python.org/dev/peps/pep-0257/) conventions.
The following examples show how to document a function parameter:

Epytext:

```pydocstring
@type param1: int
@param param1: The first parameter
```

Sphinx:

```pydocstring
:param param1: The first parameter
:type: int
```

Google:

```pydocstring
Args:
param1 (int): The first parameter
```

NumPy:

```pydocstring
Parameters
----------
param1 : int
The first parameter
```

DSP-TOOLS uses the Google style without typing,
as defined [here](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings).


## Existing formatters

### [pydocstyle](https://pypi.org/project/pydocstyle/)

Static analysis tool for checking compliance with Python docstring conventions.
Pydocstyle supports most of [PEP 257](http://www.python.org/dev/peps/pep-0257/) out of the box,
but it should not be considered a reference implementation.
Pydocstyle seems to be the most popular docstring checker.
It supports the styles "pep257", "numpy", and "google".

### [pydocstringformatter](https://pypi.org/project/pydocstringformatter/)

A docstring formatter that follows
[PEP 8](http://www.python.org/dev/peps/pep-0008/) and [PEP 257](http://www.python.org/dev/peps/pep-0257/)
but makes some of the more controversial elements of the PEPs optional.
Can be configured for other styles as well.
This project is heavily inspired by docformatter.
Supported styles: "pep257", "numpy".

### [docformatter](https://pypi.org/project/docformatter/)

Automatically formats docstrings to follow a subset of
the [PEP 257](http://www.python.org/dev/peps/pep-0257/) conventions.
Currently, only the style "sphinx" and "epytext" are recognized,
but "numpy" and "google" are future styles.
33 changes: 33 additions & 0 deletions docs/developers/code-quality-tools/python-formatting.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, this should not be in our documentation

@@ -0,0 +1,33 @@
[![PyPI version](https://badge.fury.io/py/dsp-tools.svg)](https://badge.fury.io/py/dsp-tools)

# Python formatting

There is a variety of style checkers (tools that give a feedback)
and autoformatters (tools that are able to fix the formatting violations automatically).

## Existing type checkers and autoformatters

### [pycodestyle](https://pypi.org/project/pycodestyle/)

Checks Python code against some of the style conventions in [PEP 8](http://www.python.org/dev/peps/pep-0008/).

### [autopep8](https://pypi.org/project/autopep8/)

Automatically fixes most of the formatting issues reported by pycodestyle.
Since PEP 8 is rather liberal, autopep8/pycodestyle don't modify code too much.

### [black](https://pypi.org/project/black/)

A PEP 8 compliant opinionated autoformatter with its own style, going further than autopep8/pycodestyle.
Style configuration options are deliberately limited to a minimum.
Black aims for readability and reducing git diffs.
Black is an easy to use tool, with sensible and useful defaults.
Its style is very elegant.

### [yapf](https://pypi.org/project/yapf/)

Autoformatter that can be configured to support different styles.

### [isort](https://pypi.org/project/isort/)

Sorts imports alphabetically, and separates them into sections, according to their type.