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

feat: Convert values of resolved options #125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
51 changes: 42 additions & 9 deletions src/glue_utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
from __future__ import annotations

import sys
from dataclasses import dataclass, fields
from dataclasses import Field, dataclass, fields
from typing import Any, get_type_hints
from warnings import warn

from awsglue.utils import getResolvedOptions
from typing_extensions import Self
Expand All @@ -15,20 +14,21 @@ class UnsupportedTypeWarning(UserWarning):
"""Warning for unsupported field types."""


class UnsupportedTypeError(TypeError):
"""Error raised when field type is not supported."""


@dataclass
class BaseOptions:
"""Dataclass for storing resolved options."""

@classmethod
def __init_subclass__(cls) -> None:
"""Warn if fields are not strings."""
"""Raise error when using unsupported types."""
for name, type_hint in get_type_hints(cls).items():
if type_hint is not str:
warn(
f'"{name}" value is a string at runtime even if annotated to be "{type_hint}".',
UnsupportedTypeWarning,
stacklevel=2,
)
if type_hint not in cls.SUPPORTED_TYPES:
msg = f"Field {name} has unsupported type annotation: {type_hint}"
raise UnsupportedTypeError(msg)
super().__init_subclass__()

@classmethod
Expand All @@ -51,3 +51,36 @@ def from_options(cls, options: dict[str, Any] | None = None) -> Self:
return cls(
**{key: value for key, value in options.items() if key in field_names},
)

def __post_init__(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (complexity): Consider consolidating the type conversion logic into a single method with simplified boolean handling.

The type conversion logic can be simplified while maintaining functionality:

  1. Merge the conversion logic into __post_init__
  2. Simplify boolean conversion to standard true/false strings
  3. Move type validation to __init_subclass__
def __post_init__(self) -> None:
    """Convert field values to their annotated types."""
    type_hints = get_type_hints(self)

    for field in fields(self):
        value = getattr(self, field.name)
        target_type = type_hints.get(field.name, Any)

        if not isinstance(value, target_type):
            try:
                if target_type is bool:
                    converted = str(value).lower() == "true"
                else:
                    converted = target_type(value)
                setattr(self, field.name, converted)
            except (ValueError, TypeError) as e:
                msg = f"Could not convert field {field.name} value '{value}' to {target_type.__name__}"
                raise ValueError(msg) from e

This maintains all functionality while:

  • Removing unnecessary method abstraction
  • Simplifying boolean conversion
  • Reducing nesting levels
  • Keeping error handling intact

"""Convert field values to their annotated types."""
type_hints = get_type_hints(self)

for field in fields(self):
value = getattr(self, field.name)
target_type = type_hints.get(field.name, Any)

if not isinstance(value, target_type):
self._convert_and_set_field_value(field, value, target_type)

def _convert_and_set_field_value(
self,
field: Field[Any],
value: Any, # noqa: ANN401
target_type: type,
) -> None:
"""Convert and set the field value to the target type."""
if target_type not in {str, int, float, bool}:
msg = f"Field {field.name} has unsupported type annotation: {target_type}"
raise UnsupportedTypeError(msg)

try:
if target_type is bool:
# Special handling for boolean strings
converted = str(value).lower() in ("true", "t", "yes", "y", "1")
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (code-quality): Use set when checking membership of a collection of literals (collection-into-set)

Suggested change
converted = str(value).lower() in ("true", "t", "yes", "y", "1")
converted = str(value).lower() in {"true", "t", "yes", "y", "1"}

else:
converted = target_type(value)
setattr(self, field.name, converted)
except (ValueError, TypeError) as e:
msg = f"Could not convert field {field.name} value '{value}' to {target_type.__name__}"
raise ValueError(msg) from e
Loading