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

refactor: stricter linting rules #230

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
8 changes: 4 additions & 4 deletions docs/api/treatment_effect.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ The `TreatmentEffect` module is designed for analyzing treatment effects within
:toctree: _autosummary
:recursive:

medmodels.treatment_effect.treatment_effect.TreatmentEffect
medmodels.treatment_effect.builder.TreatmentEffectBuilder
medmodels.treatment_effect.estimate.Estimate
medmodels.treatment_effect.report.Report
medmodels.treatment_effect.treatment_effect
medmodels.treatment_effect.builder
medmodels.treatment_effect.estimate
medmodels.treatment_effect.report
medmodels.treatment_effect.temporal_analysis
```
3 changes: 2 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import sys

Check failure on line 1 in docs/conf.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (D100)

docs/conf.py:1:1: D100 Missing docstring in public module
from datetime import date
from pathlib import Path

Expand Down Expand Up @@ -69,6 +69,7 @@
"private-members": False,
"inherited-members": True,
"show-inheritance": True,
"ignore-module-all": False,
}

autosummary_generate = True
Expand Down Expand Up @@ -170,7 +171,7 @@


# Local Sphinx extensions
def setup(app: Sphinx):
def setup(app: Sphinx) -> None:
"""Add custom directives and transformations to Sphinx."""
from myst_parser._docs import (
DirectiveDoc,
Expand Down
58 changes: 27 additions & 31 deletions docs/developer_guide/docstrings.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,14 @@ The summary line provides a brief description of what the function or class does

Example:

```python
def example_function(param1, param2):
"""This function performs an example operation.

Args:
param1 (int): The first parameter.
param2 (str): The second parameter.

Returns:
bool: True if successful, False otherwise.
"""
```{literalinclude} example_docstrings.py
:lines: 1-5, 6-38
:emphasize-lines: 13
```

:::{note}
The summary line cannot contain line breaks. When writing docstrings, the max line length setting can be exceeded without triggering linting errors.
:::{admonition} No Linebreaks In Summary Line
:type: note
The summary line cannot contain line breaks and must not exceed the maximum line length. Ensure that the summary line provides a brief description of the function's purpose. Further explanation and details can be given in the [Description](#description) text block underneath.
:::

### Description
Expand Down Expand Up @@ -221,24 +214,24 @@ Document parameters under the `Args` section. Each parameter should include its

Example:

```python
def example_function(param1, param2, param3):
"""This function performs an example operation.

Args:
param1 (int): The first parameter.
param2 (Union[str, List[str], Dict[str, Any]]): The second parameter, which can be
a string, a list of strings, or a dictionary with string keys and any type of
values. This parameter is used to demonstrate a long type definition.
param3 (str): The third parameter, which has a long description that needs to be
broken into multiple lines for better readability. This parameter is used
to show how to write long descriptions and how to indent them properly in
the docstring.
"""
```{literalinclude} example_docstrings.py
:lines: 1-5, 6-38
:emphasize-lines: 18-26
```

:::{note}
Type definitions of parameters cannot have a line break. You can not put a line break inside `(Union[str, List[str], Dict[str, Any]])`. When writing docstrings, the max line length setting can be exceeded without triggering linting errors.
:::{admonition} No Linebreak in Type Definitions
:type: note

When writing type definitions in argument docstrings, avoid placing line breaks inside the type annotations. For instance, complex types like `(Union[str, List[str], Dict[str, Any]])` should appear on a single line without splitting across lines. This ensures the type definition remains clear and avoids parsing issues.

If a type definition exceeds the maximum line length limit, deactivate the line length rule for that doctstring using [`# noqa: E501`](https://docs.astral.sh/ruff/rules/line-too-long/#error-suppression) after the closing `"""`. This keeps the type annotation intact while preventing the linter from flagging the long line as an error.
:::

:::{admonition} No Boolean Argument Types
:type: note
Avoid using booleans as function arguments due to the "boolean trap". Booleans reduce code clarity, as `True` or `False` doesn't explain meaning. They also limit future flexibility — if more than two options are needed, changing the argument type can cause breaking changes. Instead, use an enum or a more descriptive type for better clarity and flexibility.

For more information, you can refer to [Adam Johnson's article](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) which discusses this in detail and provides examples of how to avoid the boolean trap.
:::

### Return Types
Expand All @@ -256,6 +249,11 @@ def example_function(param1, param2):
"""
```

:::{admonition} Don't Document `None` Return Value
:type: note
Don't add a `Returns` section when the function has no return value (`def fun() -> None`)
:::

### Examples

Provide examples under the `Examples` section by using Sphinx's `.. code-block::` directive within the docstrings.
Expand Down Expand Up @@ -286,8 +284,6 @@ The second block shows the return value when executing the code. The output valu

```



Full Example:

```python
Expand Down
60 changes: 60 additions & 0 deletions docs/developer_guide/example_docstrings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
"""Example module with docstrings for the developer guide."""

from __future__ import annotations

from typing import Any, Dict, Iterator, List, Optional, Tuple, Union


def example_function_args(
param1: int,
param2: Union[str, int],
optional_param: Optional[List[str]] = None,
*args: Union[float, str],
**kwargs: Dict[str, Any],
) -> Tuple[bool, List[str]]:
"""Example function with PEP 484 type annotations and PEP 563 future annotations.

This function shows how to define and document typing for different kinds of
arguments, including positional, optional, variable-length args, and keyword args.

Args:
param1 (int): A required integer parameter.
param2 (Union[str, int]): A parameter that can be either a string or an integer.
optional_param (Optional[List[str]], optional): An optional parameter that
accepts a list of strings. Defaults to None if not provided.
*args (Union[float, str]): Variable length argument list that accepts floats or
strings.
**kwargs (Dict[str, Any]): Arbitrary keyword arguments as a dictionary of string
keys and values of any type.

Returns:
Tuple[bool, List[str]]: A tuple containing:
- bool: Always True for this example.
- List[str]: A list with a single string describing the received arguments.
"""
result = (
f"Received: param1={param1}, param2={param2}, optional_param={optional_param}, "
f"args={args}, kwargs={kwargs}"
)

return True, [result]


def example_generator(n: int) -> Iterator[int]:
"""Generators have a ``Yields`` section instead of a ``Returns`` section.

Args:
n (int): The upper limit of the range to generate, from 0 to `n` - 1.

Yields:
int: The next number in the range of 0 to `n` - 1.

Examples:
Examples should be written in doctest format, and should illustrate how
to use the function.

>>> print([i for i in example_generator(4)])
[0, 1, 2, 3]

"""
yield from range(n)
16 changes: 9 additions & 7 deletions docs/serve_docs.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
#!/usr/bin/env python3

Check failure on line 1 in docs/serve_docs.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (D100)

docs/serve_docs.py:1:1: D100 Missing docstring in public module

import logging
import os
import subprocess
from pathlib import Path

from livereload import Server, shell

logging.basicConfig(level=logging.INFO)

def setup_live_docs_server():
"""
Set up and run a live documentation server.

def setup_live_docs_server() -> None:
"""Set up and run a live documentation server.

This function initializes a server that automatically rebuilds and refreshes
the documentation when source files are modified.
Expand All @@ -25,7 +27,7 @@
rebuild_cmd = shell("make html", cwd=str(script_path))

# Initially build the documentation
print("Building initial documentation...")
logging.info("Building initial documentation...")
subprocess.run(["make", "html"], cwd=str(script_path), check=True)

# File patterns to watch for changes
Expand All @@ -48,7 +50,7 @@


if __name__ == "__main__":
print("Starting live documentation server...")
print("Access the docs at http://localhost:5500")
print("Press Ctrl+C to stop the server.")
logging.info("Starting live documentation server...")
logging.info("Access the docs at http://localhost:5500")
logging.info("Press Ctrl+C to stop the server.")
setup_live_docs_server()
2 changes: 0 additions & 2 deletions medmodels/_medmodels.pyi
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from __future__ import annotations

from enum import Enum
from typing import TYPE_CHECKING, Dict, List, Optional, Sequence, Union

Expand All @@ -20,7 +18,7 @@
PolarsNodeDataFrameInput,
)

if TYPE_CHECKING:

Check failure on line 21 in medmodels/_medmodels.pyi

View workflow job for this annotation

GitHub Actions / lint

Ruff (PYI002)

medmodels/_medmodels.pyi:21:4: PYI002 `if` test must be a simple comparison against `sys.platform` or `sys.version_info`
import sys

if sys.version_info >= (3, 10):
Expand Down
26 changes: 13 additions & 13 deletions medmodels/medrecord/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from medmodels.medrecord.datatype import (

Check failure on line 1 in medmodels/medrecord/__init__.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (D104)

medmodels/medrecord/__init__.py:1:1: D104 Missing docstring in public package
Any,
Bool,
DateTime,
Expand All @@ -20,23 +20,23 @@
from medmodels.medrecord.schema import AttributeType, GroupSchema, Schema

__all__ = [
"MedRecord",
"String",
"Int",
"Float",
"Any",
"AttributeType",
"Bool",
"DateTime",
"EdgeIndex",
"EdgeOperation",
"Float",
"GroupSchema",
"Int",
"MedRecord",
"NodeIndex",
"NodeOperation",
"Null",
"Any",
"Union",
"Option",
"AttributeType",
"Schema",
"GroupSchema",
"node",
"String",
"Union",
"edge",
"NodeIndex",
"EdgeIndex",
"NodeOperation",
"EdgeOperation",
"node",
]
2 changes: 1 addition & 1 deletion medmodels/medrecord/_overview.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,19 @@
)


def extract_attribute_summary(

Check failure on line 21 in medmodels/medrecord/_overview.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (D417)

medmodels/medrecord/_overview.py:21:5: D417 Missing argument description in the docstring for `extract_attribute_summary`: `attribute_dictionary`
attribute_dictionary: Union[

Check failure on line 22 in medmodels/medrecord/_overview.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (FA100)

medmodels/medrecord/_overview.py:22:27: FA100 Add `from __future__ import annotations` to simplify `typing.Union`
Dict[EdgeIndex, Attributes], Dict[NodeIndex, Attributes]
],
schema: Optional[AttributesSchema] = None,

Check failure on line 25 in medmodels/medrecord/_overview.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (FA100)

medmodels/medrecord/_overview.py:25:13: FA100 Add `from __future__ import annotations` to simplify `typing.Optional`
) -> Dict[
MedRecordAttribute,
Union[TemporalAttributeInfo, NumericAttributeInfo, StringAttributeInfo],

Check failure on line 28 in medmodels/medrecord/_overview.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (FA100)

medmodels/medrecord/_overview.py:28:5: FA100 Add `from __future__ import annotations` to simplify `typing.Union`
]:
"""Extracts a summary from a node or edge attribute dictionary.

Args:
attribute_dict (Union[Dict[EdgeIndex, Attributes], Dict[NodeIndex, Attributes]]):

Check failure on line 33 in medmodels/medrecord/_overview.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (W505)

medmodels/medrecord/_overview.py:33:89: W505 Doc line too long (89 > 88)
Edges or Nodes and their attributes and values.
schema (Optional[AttributesSchema], optional): Attribute Schema for the group
nodes or edges. Defaults to None.
Expand Down Expand Up @@ -184,7 +184,7 @@

info_order = ["min", "max", "mean", "values"]

for group in data.keys():
for group in data:
# determine longest group name and count
lengths[0] = max(len(str(group)), lengths[0])

Expand Down
7 changes: 5 additions & 2 deletions medmodels/medrecord/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
if TYPE_CHECKING:
from typing_extensions import TypeIs

from medmodels.medrecord.schema import Schema

import medmodels as mm
from medmodels.medrecord.schema import Schema
from medmodels.medrecord.types import (
EdgeTuple,
Group,
Expand Down Expand Up @@ -157,7 +158,7 @@ def add_edges(
return self

def add_group(
self, group: Group, *, nodes: List[NodeIndex] = []
self, group: Group, *, nodes: Optional[List[NodeIndex]] = None
) -> MedRecordBuilder:
"""Adds a group to the builder with an optional list of nodes.

Expand All @@ -168,6 +169,8 @@ def add_group(
Returns:
MedRecordBuilder: The current instance of the builder.
"""
if nodes is None:
nodes = []
self._groups[group] = {"nodes": nodes, "edges": []}
return self

Expand Down
26 changes: 13 additions & 13 deletions medmodels/medrecord/datatype.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

import typing
from abc import ABCMeta, abstractmethod
from abc import ABC, abstractmethod

from medmodels._medmodels import (
PyAny,
Expand Down Expand Up @@ -36,7 +36,7 @@
]


class DataType(metaclass=ABCMeta):
class DataType(ABC):
@abstractmethod
def _inner(self) -> PyDataType: ...

Expand All @@ -53,25 +53,24 @@ def __eq__(self, value: object) -> bool: ...
def _from_pydatatype(datatype: PyDataType) -> DataType:
if isinstance(datatype, PyString):
return String()
elif isinstance(datatype, PyInt):
if isinstance(datatype, PyInt):
return Int()
elif isinstance(datatype, PyFloat):
if isinstance(datatype, PyFloat):
return Float()
elif isinstance(datatype, PyBool):
if isinstance(datatype, PyBool):
return Bool()
elif isinstance(datatype, PyDateTime):
if isinstance(datatype, PyDateTime):
return DateTime()
elif isinstance(datatype, PyNull):
if isinstance(datatype, PyNull):
return Null()
elif isinstance(datatype, PyAny):
if isinstance(datatype, PyAny):
return Any()
elif isinstance(datatype, PyUnion):
if isinstance(datatype, PyUnion):
return Union(
DataType._from_pydatatype(datatype.dtype1),
DataType._from_pydatatype(datatype.dtype2),
)
else:
return Option(DataType._from_pydatatype(datatype.dtype))
return Option(DataType._from_pydatatype(datatype.dtype))


class String(DataType):
Expand Down Expand Up @@ -212,8 +211,9 @@ class Union(DataType):

def __init__(self, *dtypes: DataType) -> None:
if len(dtypes) < 2:
raise ValueError("Union must have at least two arguments")
elif len(dtypes) == 2:
msg = "Union must have at least two arguments"
raise ValueError(msg)
JabobKrauskopf marked this conversation as resolved.
Show resolved Hide resolved
if len(dtypes) == 2:
self._union = PyUnion(dtypes[0]._inner(), dtypes[1]._inner())
else:
self._union = PyUnion(dtypes[0]._inner(), Union(*dtypes[1:])._inner())
Expand Down
Loading
Loading