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

Support generic types in config fields #465

Open
steinitzu opened this issue Jun 30, 2023 · 0 comments
Open

Support generic types in config fields #465

steinitzu opened this issue Jun 30, 2023 · 0 comments

Comments

@steinitzu
Copy link
Collaborator

Currently we don't support TypeVars in config fields, so generic types need to be defined Any

We should allow typevar hints on fields which are part of generic classes.

This would allow incremental config to be typed and validated correctly:

TCursorValue = TypeVar("TCursorValue", bound=Any)

@configspec
class Incremental(BaseConfiguration, Generic[TCursorValue]):
    cursor_path: str = None
    initial_value: Optional[TCursorValue] = None

...

@configspec
class SomeResourceConfig(BaseConfiguration):
    updated_at: Incremental[pendulum.DateTime]

Implementation notes:

  • base_configuration.is_valid_hint should allow TypeVar as a hint

    Best would be to check whether the spec class is a generic with the same typevar and not allow unbound typevars.
    E.g. this is fine:

    class Incremental(BaseConfiguration, Generic[T]):
        initial_value: Optional[T]
    

    This is not

    class Incremental(BaseConfiguration):
        initial_value: Optional[T]
    
  • get_resolvable_fields when called on a concrete subclass or a generic alias, such as Incremental[str] or class SomeClass(Incremental[str])

    Should match the typevar of the fields with the typevars on the generic base and return the concrete type for each field. There may be multiple generic types in the class.

  • When get_resolvable_fields is called on the generic baseclass, if typevars can't be mapped to concrete types, either:

    1. Raise an exception -> force user to specify concrete type (may be breaking change)
    2. Allow it but replace unmatched typevar hints with Any
    3. Start with 2. but with deprecation warnings and change to 1. in later release

Testing

  • All existing tests should pass unchanged
  • Test generic with 1 and multiple typevars
  • Test generic baseclass as a config
  • Test subclass of a concrete type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

1 participant