Skip to content

Commit

Permalink
[PYG-268] 😏 Capture warnings (#347)
Browse files Browse the repository at this point in the history
* refactor: shell for new warning handling

* refactor; print first one

* refactor; consisten naming

* feat; improved print

* fix: proper group name

* style: remove

* build: changelog

* tests: fix tests

* refactor: pygen warnings are now handleg internally

* tests: updated test
  • Loading branch information
doctrino authored Nov 9, 2024
1 parent 34ca325 commit cf8d41a
Show file tree
Hide file tree
Showing 12 changed files with 262 additions and 112 deletions.
8 changes: 2 additions & 6 deletions cognite/pygen/_core/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from pydantic.version import VERSION as PYDANTIC_VERSION

from cognite.pygen._version import __version__
from cognite.pygen._warnings import PydanticNamespaceCollisionWarning
from cognite.pygen.config import PygenConfig

from . import validation
Expand Down Expand Up @@ -833,12 +834,7 @@ def generate_data_class_file(self) -> str:

if self.data_class.has_any_field_model_prefix:
names = ", ".join(field.name for field in self.data_class.fields if field.name.startswith("name"))
warnings.warn(
f"Field(s) {names} in view {self.view_id} has potential conflict with protected Pydantic "
"namespace 'model_'",
UserWarning,
stacklevel=2,
)
warnings.warn(PydanticNamespaceCollisionWarning(self.view_id, names), stacklevel=2)

return (
type_data.render(
Expand Down
8 changes: 3 additions & 5 deletions cognite/pygen/_core/models/fields/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
SingleReverseDirectRelation,
)

from cognite.pygen.warnings import MissingReverseDirectRelationTargetWarning
from cognite.pygen._warnings import MissingReverseDirectRelationTargetWarning

from .base import Field

Expand Down Expand Up @@ -251,17 +251,15 @@ def load(
if not isinstance(prop, dm.EdgeConnection | dm.MappedProperty | ReverseDirectRelation):
return None
if isinstance(prop, ReverseDirectRelation):
field_string = f"{view_id}.{base.prop_name}"
target = (
direct_relations_by_view_id.get(prop.through.source)
if isinstance(prop.through.source, dm.ViewId)
else None
)
if target is None or prop.through.property not in target:
target_str = (
f"{prop.through.source}.{prop.through.property}" if target is not None else str(prop.through.source)
warnings.warn(
MissingReverseDirectRelationTargetWarning(prop.through, view_id, base.prop_name), stacklevel=2
)
warnings.warn(MissingReverseDirectRelationTargetWarning(target_str, field_string), stacklevel=2)
return None
edge_type = prop.type if isinstance(prop, dm.EdgeConnection) else None
direction: Literal["outwards", "inwards"]
Expand Down
40 changes: 37 additions & 3 deletions cognite/pygen/_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
from cognite.pygen._core.models import DataClass
from cognite.pygen._settings import _load_pyproject_toml
from cognite.pygen._version import __version__
from cognite.pygen._warnings import InvalidCodeGenerated, print_warnings
from cognite.pygen.config import PygenConfig
from cognite.pygen.exceptions import DataModelNotFound
from cognite.pygen.utils.text import to_pascal, to_snake
from cognite.pygen.warnings import InvalidCodeGenerated

DataModel = Union[DataModelIdentifier, dm.DataModel[dm.View]]

Expand Down Expand Up @@ -101,6 +101,35 @@ def generate_sdk(
return_sdk_files: Whether to return the generated SDK files as a dictionary. Defaults to False.
This is useful for granular control of how to write the SDK to disk.
"""
return _generate_sdk(
model_id,
client,
top_level_package,
client_name,
default_instance_space,
output_dir,
logger,
overwrite,
format_code,
config,
return_sdk_files,
)


def _generate_sdk(
model_id: DataModel | Sequence[DataModel],
client: Optional[CogniteClient] = None,
top_level_package: Optional[str] = None,
client_name: Optional[str] = None,
default_instance_space: str | None = None,
output_dir: Optional[Path] = None,
logger: Optional[Callable[[str], None]] = None,
overwrite: bool = False,
format_code: bool = True,
config: Optional[PygenConfig] = None,
return_sdk_files: bool = False,
context: Literal["notebook", "cli"] = "cli",
) -> None | dict[Path, str]:
if output_dir is not None and top_level_package is None:
raise ValueError("top_level_package must be provided if output_dir is provided")
logger = logger or print
Expand All @@ -122,7 +151,11 @@ def generate_sdk(
logger,
config or PygenConfig(),
)
sdk = sdk_generator.generate_sdk()
with warnings.catch_warnings(record=True) as warning_logger:
sdk = sdk_generator.generate_sdk()
if warning_logger:
print_warnings(warning_logger, logger, context)

if return_sdk_files:
return sdk
output_dir = output_dir or (Path.cwd() / Path(top_level_package.replace(".", "/")))
Expand Down Expand Up @@ -191,7 +224,7 @@ def generate_sdk_notebook(
top_level_package = _default_top_level_package(external_id)
if client_name is None:
client_name = _default_client_name(external_id)
generate_sdk(
_generate_sdk(
data_model,
client,
top_level_package=top_level_package,
Expand All @@ -201,6 +234,7 @@ def generate_sdk_notebook(
overwrite=True,
format_code=False,
config=config,
context="notebook",
)
if str(output_dir) not in sys.path:
sys.path.append(str(output_dir))
Expand Down
180 changes: 180 additions & 0 deletions cognite/pygen/_warnings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
from __future__ import annotations

import itertools
import warnings
from collections.abc import Callable
from pathlib import Path
from typing import Literal, cast

from cognite.client.data_classes.data_modeling import PropertyId, ViewId


class PygenWarning(UserWarning):
"""Base class for warnings in pygen."""

def warn(self):
warnings.warn(self, stacklevel=2)


class NameCollisionWarning(PygenWarning, RuntimeWarning):
@classmethod
def create(
cls,
word: str,
word_type: Literal["field", "data class", "parameter", "filename"],
view_id: ViewId | None,
property_name: str | None,
) -> NameCollisionWarning:
if view_id and property_name:
return NameCollisionViewPropertyWarning(view_id, property_name, word)
elif view_id and word_type == "filename":
return NameCollisionFileNameWarning(view_id, word)
elif view_id and word_type == "data class":
return NameCollisionDataClassNameWarning(view_id, word)
else:
return NameCollisionParameterWarning(word)


class NameCollisionFileNameWarning(NameCollisionWarning):
def __init__(self, view_id: ViewId, word: str):
self.view_id = view_id
self.word = word

def __str__(self) -> str:
return (
f"Name collision detected in {self.view_id}: {self.word!r}. "
f"An underscore will be added to the {self.word!r} to avoid name collision."
)


class NameCollisionDataClassNameWarning(NameCollisionWarning):
def __init__(self, view_id: ViewId, word: str):
self.view_id = view_id
self.word = word

def __str__(self) -> str:
return (
f"Name collision detected in {self.view_id}: {self.word!r}. "
f"An underscore will be added to the {self.word!r} to avoid name collision."
)


class NameCollisionViewPropertyWarning(NameCollisionWarning):
def __init__(self, view_id: ViewId, property_name: str, word: str):
self.view_id = view_id
self.property_name = property_name
self.word = word

def __str__(self) -> str:
return (
f"Name collision detected in {self.view_id}: {self.property_name!r}. "
f"An underscore will be added to the {self.word!r} to avoid name collision."
)


class NameCollisionParameterWarning(NameCollisionWarning):
def __init__(self, word: str):
self.word = word

def __str__(self) -> str:
return (
f"Name collision detected. The following filter parameter {self.word!r} name is used by pygen."
"An underscore will be added to this parameter to avoid name collision."
)


class MissingReverseDirectRelationTargetWarning(PygenWarning, UserWarning):
def __init__(self, source: PropertyId, view: ViewId, property_: str) -> None:
self.source = source
self.view = view
self.property_ = property_

def __str__(self) -> str:
return (
f"Target {self.source!r} does not exists. "
f"Skipping reverse direct relation {self.view.external_id}.{self.property_}."
)


class InvalidCodeGenerated(PygenWarning, UserWarning):
def __init__(self, filepath: str | Path, error_message: str) -> None:
self.filepath = filepath
self.error_message = error_message

def __str__(self) -> str:
return f"Invalid code generated in {self.filepath}. {self.error_message}"


class PydanticNamespaceCollisionWarning(PygenWarning, UserWarning):
def __init__(self, view_id: ViewId, names: str) -> None:
self.view_id = view_id
self.names = names

def __str__(self) -> str:
return (
f"Field(s) {self.names} in view {self.view_id} has potential conflict "
"with protected Pydantic namespace 'model_'"
)


def print_warnings(
warning_list: list[warnings.WarningMessage],
console: Callable[[str], None],
context: Literal["notebook", "cli"] = "cli",
) -> None:
for group, group_warnings in itertools.groupby(
sorted(warning_list, key=lambda w: w.category.__name__),
key=lambda w: w.category,
):
group_list = [w.message for w in group_warnings if isinstance(w.message, PygenWarning)]
_print_warning_group(group_list, group, console, context)


def _print_warning_group(
pygen_warnings: list[PygenWarning],
group: type[PygenWarning],
console: Callable[[str], None],
context: Literal["notebook", "cli"],
) -> None:
if group is PydanticNamespaceCollisionWarning:
return
elif context == "notebook" and group is NameCollisionFileNameWarning:
return
elif group is InvalidCodeGenerated or len(pygen_warnings) == 1:
_print_one_by_one(console, pygen_warnings)
elif issubclass(group, NameCollisionWarning | MissingReverseDirectRelationTargetWarning):
_print_group(console, group, pygen_warnings)


def _print_one_by_one(console: Callable[[str], None], warning_list: list[PygenWarning]) -> None:
for warning in warning_list:
console(f"{warning!s}")


def _print_group(console: Callable[[str], None], group: type[PygenWarning], warning_list: list[PygenWarning]) -> None:
console(f"{group.__name__}: {len(warning_list)}")
indent = " " * 4
if group in {NameCollisionFileNameWarning, NameCollisionDataClassNameWarning}:
view_warnings = cast(list[NameCollisionFileNameWarning | NameCollisionDataClassNameWarning], warning_list)
views_str = ", ".join(f"{warning.view_id!r}" for warning in view_warnings)
console(f"{indent}The following views will have an underscore added to avoid name collision: {views_str}")
elif group is NameCollisionViewPropertyWarning:
property_warnings = cast(list[NameCollisionViewPropertyWarning], warning_list)
for view, properties in itertools.groupby(
sorted(property_warnings, key=lambda w: w.view_id.external_id), key=lambda w: w.view_id.external_id
):
properties_str = ", ".join(warning.property_name for warning in properties)
console(
f"{indent}The following properties in view {view} will have an underscore "
f"added to avoid name collision: {properties_str}"
)
elif group is MissingReverseDirectRelationTargetWarning:
relation_warnings = cast(list[MissingReverseDirectRelationTargetWarning], warning_list)
for relation_warn in relation_warnings:
console(
f"{indent} Skipping reverse direct "
f"relation {relation_warn.view.external_id}.{relation_warn.property_}."
)
else:
for warning in warning_list:
console(f"{indent}{warning!s}")
4 changes: 2 additions & 2 deletions cognite/pygen/config/reserved_words.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from cognite.client import data_modeling as dm
from pydantic import BaseModel

from cognite.pygen.warnings import NameCollisionWarning
from cognite.pygen._warnings import NameCollisionWarning

PYTHON_BUILTIN_NAMES = {name for name in vars(builtins) if not name.startswith("_")}
FIELD_NAMES = (
Expand Down Expand Up @@ -75,6 +75,6 @@ def is_reserved_word(
property_name: str | None = None,
) -> bool:
if keyword.iskeyword(word) or word in PYTHON_BUILTIN_NAMES or word in _NAMES_BY_TYPE[word_type]:
NameCollisionWarning.create(word, view_id, property_name).warn()
NameCollisionWarning.create(word, word_type, view_id, property_name).warn()
return True
return False
Loading

0 comments on commit cf8d41a

Please sign in to comment.