-
Notifications
You must be signed in to change notification settings - Fork 90
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 ModelBuilder
and RandomBuilder
#971
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,50 @@ | ||
from __future__ import annotations | ||
|
||
import datetime | ||
import enum | ||
import random | ||
import string | ||
import typing as t | ||
import uuid | ||
from decimal import Decimal | ||
from functools import partial | ||
from uuid import UUID | ||
|
||
from piccolo.columns import Array, Column | ||
|
||
|
||
class RandomBuilder: | ||
@classmethod | ||
def _build(cls, column: Column) -> t.Any: | ||
if e := column._meta.choices: | ||
return cls.next_enum(e) | ||
dantownsend marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
mapper: t.Dict[t.Type, t.Callable] = { | ||
bool: cls.next_bool, | ||
bytes: cls.next_bytes, | ||
datetime.date: cls.next_date, | ||
datetime.datetime: partial( | ||
cls.next_datetime, getattr(column, "tz_aware", False) | ||
), | ||
float: cls.next_float, | ||
Decimal: partial( | ||
cls.next_decimal, column._meta.params.get("digits") | ||
), | ||
dantownsend marked this conversation as resolved.
Show resolved
Hide resolved
|
||
int: cls.next_int, | ||
str: partial(cls.next_str, column._meta.params.get("length")), | ||
datetime.time: cls.next_time, | ||
datetime.timedelta: cls.next_timedelta, | ||
UUID: cls.next_uuid, | ||
} | ||
|
||
random_value_callable = mapper.get(column.value_type) | ||
if random_value_callable is None: | ||
random_value_callable = partial( | ||
cls.next_list, | ||
mapper[t.cast(Array, column).base_column.value_type], | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if anything besides an It wonder if we can add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The handling of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right - arrays are always tricky edge cases. Maybe we could add a check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my latest comment regarding this issue. |
||
return random_value_callable() | ||
|
||
@classmethod | ||
def next_bool(cls) -> bool: | ||
return random.choice([True, False]) | ||
|
@@ -43,12 +81,18 @@ def next_enum(cls, e: t.Type[enum.Enum]) -> t.Any: | |
def next_float(cls, minimum=0, maximum=2147483647, scale=5) -> float: | ||
return round(random.uniform(minimum, maximum), scale) | ||
|
||
@classmethod | ||
def next_decimal(cls, digits: t.Tuple[int, int] | None = (4, 2)) -> float: | ||
precision, scale = digits or (4, 2) | ||
return cls.next_float(maximum=10 ** (precision - scale), scale=scale) | ||
dantownsend marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@classmethod | ||
def next_int(cls, minimum=0, maximum=2147483647) -> int: | ||
return random.randint(minimum, maximum) | ||
|
||
@classmethod | ||
def next_str(cls, length=16) -> str: | ||
def next_str(cls, length: int | None = 16) -> str: | ||
length = length or 16 | ||
dantownsend marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return "".join( | ||
random.choice(string.ascii_letters) for _ in range(length) | ||
) | ||
|
@@ -72,3 +116,8 @@ def next_timedelta(cls) -> datetime.timedelta: | |
@classmethod | ||
def next_uuid(cls) -> uuid.UUID: | ||
return uuid.uuid4() | ||
|
||
@classmethod | ||
def next_list(cls, callable_: t.Callable) -> t.List[t.Any]: | ||
length = cls.next_int(maximum=10) | ||
return [callable_() for _ in range(length)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about calling this something like
get_value_for_column
? Do you think it should be a private method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially considered treating it as a private method, but also wanted to find a way to provide a mechanism for registering a new type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be nice.