Skip to content

Commit

Permalink
Developer Documentation: Brief style guidelines and instructions for …
Browse files Browse the repository at this point in the history
…checking style rucio#287
  • Loading branch information
voetberg committed Apr 2, 2024
1 parent 358b0f0 commit 732e997
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 0 deletions.
171 changes: 171 additions & 0 deletions docs/developer/style_guide.md
@@ -0,0 +1,171 @@
---
id: dev_style_guide
title: Style Guide
---

**TL;DR** - Install the provided pre-commits, follow their recommendations

# General Style
## Spacing
* Include 2 lines between functions outside of classes.
* Include 1 line between class methods.
* Include 2 lines between the import block and main code body.

## White Space
* Puncuation requires trailing whitespace except at the end of a line, and no leading whitespace.
* Assignement (`=`) or comparision (`==`, `<`, `in`, etc) required surrounding white space, except when used in default definitions.
* Colons in dictionaries or type annotations should have trailing, but not leading whitespace.

## Imports
* Never import using `from x import *`
* Order alphabetically, then seperated into sections for internal and external dependencies. Group internal imports at the end of the block, and group imports from the same external package.
* Order modules such that `import {packageA}` is before `from {packageB} import {Module}`
* Do not import whole packages when single modules would suffice.
* Unused imports must be removed.
* When a large number of individual imports form a single package/module is required, group them together with `()` and separate them on their own lines.
* When importing a module specifically for type checking (e.g. a core module that may not be included in every distribution of rucio, a type from SQLAlchemy), contain them in a block using
```python
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from {package} import {module}
```

[`ruff's isort` implimentation](https://docs.astral.sh/ruff/faq/#how-does-ruffs-import-sorting-compare-to-isort) handles import sorting in the rucio `pre-commit`s.

#### Examples:
```python
# Wrong
import rucio
from datetime import *
import os

# Right
import os
from datetime import datetime, timedelta

from rucio.core.did import add_did
```

```python
# Wrong
from packageA import moduleA, moduleB, moduleC, moduleD, moduleE, moduleF, moduleG, ...

# Right
from packageA import (
moduleA,
moduleB,
moduleC,
moduleD,
moduleE,
moduleF,
moduleG,
...
)
```
## Docstrings
TBA

# Testing Guide
* Use fixtures (found in the tests/conftest.py) or temporary object factories (tests/temp_factories.py) instead of making bare instances of rucio objects.
* Only write tests derministically. Randomness produces [flaky tests](https://docs.pytest.org/en/7.1.x/explanation/flaky.html).
* Only write tests that are "stand alone" - tests should be entirely self-contained besides for the beforementioned fixtures and factories.
* If a test requires a configuration file changed, [use a fixture to modify a mock-configuration file.](https://github.com/rucio/rucio/blob/master/tests/conftest.py#L510)
* If a test can interfer with another test (use the same database table, interact with a queue), mark it as `noparallel`.

# SQLAlchemy Query Guide
Rucio uses a custom SQLAlchemy style.
Each major clause or operation is given a new line, and a level of indentation is added for each operation within a part of a query.

When queries as executed, perfered output types are either scalars or single values, to avoid using an index to select an element out of a query.

Shorter statements can be written on a single line.

> **Note** - When using SQLAlchemy to make query data model, it is best practice to call the code executed (the `select` statment or similar) `statement` or `stmt`. The actual `query` is the result of the statement's execution.
### Examples
#### Simple Select
```python
statement = select(
models.Table.ColumnA,
models.Table.ColumnB,
)
query = session.execute(statement).scalars()
for column_a, column_b in query:
...
```
#### Select with Join
```python
statement = select(
models.Table1.ColumnA,
models.Table1.ColumnB,
models.Table2.Column2
).join(
models.Table2,
(models.Table1.keyA == models.Table2.keyA) & (models.Table1.keyB == models.Table2.keyB)
)
query = session.execute(statement).scalars()
for column_a, column_b, column_c in query:
...
```

#### Multiple Conditions
```python
statement = select(
models.Table1.Column_a,
models.Table1.Column_b,
models.Table1.Column_c,
).where(
and_(
models.Table1.Condition1 == True,
models.Table1.Condition2 != None,
models.Table1.Condition2 < {Different Statement}
)
)
query = session.execute(statement).scalars()
for column_a, column_b, column_c in query:
...
```

#### Checking existance/Single Value
When using the query to ensure that table has been populated, or only a single result is required, use either `session.execute().one()` or `session.execute().scalar_one()`.

```python
statement = select(model.Table.Column).where(Condition)
try:
session.execute(statement).one()
except NoResultFound as e:
... # Handle the case where nothing exists
```

```python
statement = select(model.Table).where(Condition)
query = session.execute(statement).scalar_one()
foo(query.Column1, query.Column2)
```

# Pre-commits
Rucio uses [`flake8`](https://flake8.pycqa.org/en/latest/) as a linter, [`ruff`](https://docs.astral.sh/ruff/) as a formatter, a custom write-space remover,
and a script to verify a uniform file-header format.
Please use these before submitting a pull request.

The Rucio repo provides a `pre-commit` that does this automatically.
Install it with the below commands.

```shell
pip install pre-commit
pre-commit install
```

# Github Actions
Code style is checked during a pull request with a github action.
The action checks the header and type annotations (including a count and veracity).
More information about type annotations can be found [here](./type_annotation_guide.md).
These checks can also be run locally using

```shell
tools/count_missing_type_annotations_utils.sh
tools/run_pyright.sh generate {report_output_path.json}
```
The first action will raise an error if your commits introduce more un-annotated types than it solves,
and the second ensures the added types are consistent with the rest of the codebase.

1 change: 1 addition & 0 deletions website/sidebars.json
Expand Up @@ -80,6 +80,7 @@
"contributing",
"developer/rest_api_doc",
"developer/type_annotation_guide",
"developer/dev_style_guide",
{
"WebUI": [
"developer/webui/webui_frontend_vscode_dev_env",
Expand Down

0 comments on commit 732e997

Please sign in to comment.