Conversation
f1i3g3
left a comment
f1i3g3
left a comment
There was a problem hiding this comment.
Хочется, чтобы все еще раз увидели общую диаграмму - из неё следуют важные классы (без неё вообще не понять, что откуда и почему.
Так вроде всё ок.
There was a problem hiding this comment.
Мб какой-то коммент на будущее оставить для расширения?
There was a problem hiding this comment.
Для остальных перечислений так же.
There was a problem hiding this comment.
Не понял, Дим, что ты имеешь в виду.
| """ | ||
|
|
||
|
|
||
| class CriticalValueExecutionStep: |
There was a problem hiding this comment.
Не нужно ли добавить общий ExecutionStep?
There was a problem hiding this comment.
Он есть, это IExperimentStep. Он объявлен как Protocol, в котором задан метод run() c определенной сигнатурой. CriticalValueExecutionStep реализует данный метод с такой же сигнатурой, что означает, что он является подклассом IExperimentStep (duck typing). Можно было бы явно указать, но это не требуется.
There was a problem hiding this comment.
Аналогично для других шагов.
There was a problem hiding this comment.
Да, для них работает то же самое правило.
| G = TypeVar("G", covariant=True, bound=IExperimentStep) | ||
| E = TypeVar("E", covariant=True, bound=IExperimentStep) | ||
| R = TypeVar("R", covariant=True, bound=IExperimentStep) |
There was a problem hiding this comment.
Это же во всех версиях Питона будет работать?
There was a problem hiding this comment.
Начиная с 3.5 -- да
There was a problem hiding this comment.
Тут, как я понимаю, интерфейс обращения к бд.
There was a problem hiding this comment.
Это не интерфейс, а конкретная пустая (пока что) реализация.
| @dataclass | ||
| class ExperimentQuery(DataQuery): | ||
| experiment_type: str | ||
| storage_connection: str | ||
| run_mode: str | ||
| hypothesis: str | ||
| data_generator_type: str | ||
| executor_type: str | ||
| report_builder_type: str | ||
| sample_sizes: list[int] | ||
| monte_carlo_count: int | ||
| criteria: dict[str, list[float]] | ||
| alternatives: dict[str, list[float]] | ||
| significance_levels: list[float] | ||
|
|
There was a problem hiding this comment.
Мб в разные файлы вынести? Хотя я слишком докапываюсь.
There was a problem hiding this comment.
Можно было бы, но мне показалось, что этим классам логичнее лежать вместе
There was a problem hiding this comment.
Ну точно отдельный интерфейс для бд нужен!
There was a problem hiding this comment.
Интерфейсы для каждой бд есть (например, для SQLitePowerStorage -- IPowerStorage). Я не указал явное унаследование, поскольку все интерфейсы -- это Protocol. Однако интерфейсы для бд параметризуются конкретным типов, что хуже тайпчекается, поэтому добавил явное наследование, чтобы питону было чуть легче.
3 participants
Summary
Architecture refactoring.
Solve the issue: #55
Quick changelog
What's new?
Framework architecture refactoring. Previous functionality left unchanged.