Skip to content

Commit

Permalink
style[next]: standardize error messages. (#1386)
Browse files Browse the repository at this point in the history
- add style guide to the coding guidelines
- fix existing error messages in next
- deal with ensuing qa errorrs / test updates
- unshadow one test and fix the code it wasn't testing

Co-authored-by: Rico Häuselmann <[email protected]>
Co-authored-by: Enrique González Paredes <[email protected]>
  • Loading branch information
3 people authored Dec 13, 2023
1 parent 3f595ff commit a5b2450
Show file tree
Hide file tree
Showing 69 changed files with 571 additions and 458 deletions.
38 changes: 38 additions & 0 deletions CODING_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,44 @@ We deviate from the [Google Python Style Guide][google-style-guide] only in the
- Client code (like tests, doctests and examples) should use the above style for public FieldView API
- Library code should always import the defining module and use qualified names.

### Error messages

Error messages should be written as sentences, starting with a capital letter and ending with a period (avoid exclamation marks). Try to be informative without being verbose. Code objects such as 'ClassNames' and 'function_names' should be enclosed in single quotes, and so should string values used for message interpolation.

Examples:

```python
raise ValueError(f"Invalid argument 'dimension': should be of type 'Dimension', got '{dimension.type}'.")
```

Interpolated integer values do not need double quotes, if they are indicating an amount. Example:

```python
raise ValueError(f"Invalid number of arguments: expected 3 arguments, got {len(args)}.")
```

The double quotes can also be dropped when presenting a sequence of values. In this case the message should be rephrased so the sequence is separated from the text by a colon ':'.

```python
raise ValueError(f"unexpected keyword arguments: {', '.join(set(kwarg_names} - set(expected_kwarg_names)))}.")
```

The message should be kept to one sentence if reasonably possible. Ideally the sentence should be kept short and avoid unneccessary words. Examples:

```python
# too many sentences
raise ValueError(f"Received an unexpeted number of arguments. Should receive 5 arguments, but got {len(args)}. Please provide the correct number of arguments.")
# better
raise ValueError(f"Wrong number of arguments: expected 5, got {len(args)}.")

# less extreme
raise TypeError(f"Wrong argument type. Can only accept 'int's, got '{type(arg)}' instead.")
# but can still be improved
raise TypeError(f"Wrong argument type: 'int' expected, got '{type(arg)}'")
```

The terseness vs. helpfulness tradeoff should be more in favor of terseness for internal error messages and more in favor of helpfulness for `DSLError` and it's subclassses, where additional sentences are encouraged if they point out likely hidden sources of the problem or common fixes.

### Docstrings

We generate the API documentation automatically from the docstrings using [Sphinx][sphinx] and some extensions such as [Sphinx-autodoc][sphinx-autodoc] and [Sphinx-napoleon][sphinx-napoleon]. These follow the Google Python Style Guide docstring conventions to automatically format the generated documentation. A complete overview can be found here: [Example Google Style Python Docstrings](https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google).
Expand Down
22 changes: 15 additions & 7 deletions src/gt4py/_core/definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,23 @@

BoolScalar: TypeAlias = Union[bool_, bool]
BoolT = TypeVar("BoolT", bound=BoolScalar)
BOOL_TYPES: Final[Tuple[type, ...]] = cast(Tuple[type, ...], BoolScalar.__args__) # type: ignore[attr-defined]
BOOL_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], BoolScalar.__args__ # type: ignore[attr-defined]
)


IntScalar: TypeAlias = Union[int8, int16, int32, int64, int]
IntT = TypeVar("IntT", bound=IntScalar)
INT_TYPES: Final[Tuple[type, ...]] = cast(Tuple[type, ...], IntScalar.__args__) # type: ignore[attr-defined]
INT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], IntScalar.__args__ # type: ignore[attr-defined]
)


UnsignedIntScalar: TypeAlias = Union[uint8, uint16, uint32, uint64]
UnsignedIntT = TypeVar("UnsignedIntT", bound=UnsignedIntScalar)
UINT_TYPES: Final[Tuple[type, ...]] = cast(Tuple[type, ...], UnsignedIntScalar.__args__) # type: ignore[attr-defined]
UINT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], UnsignedIntScalar.__args__ # type: ignore[attr-defined]
)


IntegralScalar: TypeAlias = Union[IntScalar, UnsignedIntScalar]
Expand All @@ -93,7 +99,9 @@

FloatingScalar: TypeAlias = Union[float32, float64, float]
FloatingT = TypeVar("FloatingT", bound=FloatingScalar)
FLOAT_TYPES: Final[Tuple[type, ...]] = cast(Tuple[type, ...], FloatingScalar.__args__) # type: ignore[attr-defined]
FLOAT_TYPES: Final[Tuple[type, ...]] = cast(
Tuple[type, ...], FloatingScalar.__args__ # type: ignore[attr-defined]
)


#: Type alias for all scalar types supported by GT4Py
Expand Down Expand Up @@ -195,7 +203,7 @@ def dtype_kind(sc_type: Type[ScalarT]) -> DTypeKind:
if issubclass(sc_type, numbers.Complex):
return DTypeKind.COMPLEX

raise TypeError("Unknown scalar type kind")
raise TypeError("Unknown scalar type kind.")


@dataclasses.dataclass(frozen=True)
Expand Down Expand Up @@ -491,10 +499,10 @@ def __rtruediv__(self, other: Any) -> NDArrayObject:
def __pow__(self, other: NDArrayObject | Scalar) -> NDArrayObject:
...

def __eq__(self, other: NDArrayObject | Scalar) -> NDArrayObject: # type: ignore[override] # mypy want to return `bool`
def __eq__(self, other: NDArrayObject | Scalar) -> NDArrayObject: # type: ignore[override] # mypy wants to return `bool`
...

def __ne__(self, other: NDArrayObject | Scalar) -> NDArrayObject: # type: ignore[override] # mypy want to return `bool`
def __ne__(self, other: NDArrayObject | Scalar) -> NDArrayObject: # type: ignore[override] # mypy wants to return `bool`
...

def __gt__(self, other: NDArrayObject | Scalar) -> NDArrayObject: # type: ignore[misc] # Forward operator is not callable
Expand Down
8 changes: 5 additions & 3 deletions src/gt4py/next/allocators.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ def get_allocator(
elif not strict or is_field_allocator(default):
return default
else:
raise TypeError(f"Object {obj} is neither a field allocator nor a field allocator factory")
raise TypeError(
f"Object '{obj}' is neither a field allocator nor a field allocator factory."
)


@dataclasses.dataclass(frozen=True)
Expand Down Expand Up @@ -331,15 +333,15 @@ def allocate(
"""
if device is None and allocator is None:
raise ValueError("No 'device' or 'allocator' specified")
raise ValueError("No 'device' or 'allocator' specified.")
actual_allocator = get_allocator(allocator)
if actual_allocator is None:
assert device is not None # for mypy
actual_allocator = device_allocators[device.device_type]
elif device is None:
device = core_defs.Device(actual_allocator.__gt_device_type__, 0)
elif device.device_type != actual_allocator.__gt_device_type__:
raise ValueError(f"Device {device} and allocator {actual_allocator} are incompatible")
raise ValueError(f"Device '{device}' and allocator '{actual_allocator}' are incompatible.")

return actual_allocator.__gt_allocate__(
domain=common.domain(domain),
Expand Down
Loading

0 comments on commit a5b2450

Please sign in to comment.