review: добавил TODO
This commit is contained in:
parent
64ced613dd
commit
a6a7653ca5
@ -20,8 +20,11 @@ class Controller(BaseController):
|
||||
self.signal_widgets.emit(widgets)
|
||||
|
||||
def update_settings(self, settings: list[dict]) -> None:
|
||||
# TODO: Почему контроллеру просто не сообщить о медиаторе, и ему сообщать что-то через notify? Тем более, что
|
||||
# медиатор уже знает про контроллер...
|
||||
self.signal_settings.emit(settings)
|
||||
|
||||
# TODO: Объедини переключение режимов в один метод, и по входному аргументу (например 1 и 2) переключай их.
|
||||
def raport_mode(self) -> None:
|
||||
self.signal_raport_mode.emit()
|
||||
|
||||
@ -29,12 +32,18 @@ class Controller(BaseController):
|
||||
self.signal_seeking_mode.emit()
|
||||
|
||||
def open_file(self, filepath: str) -> None:
|
||||
# TODO: Где тут открытие файла?
|
||||
self.signal_open_file.emit(filepath)
|
||||
|
||||
def update_plots(self) -> None:
|
||||
self.signal_update_plots.emit()
|
||||
|
||||
def update_status(self, msg: Union[str, float, int]) -> None:
|
||||
# TODO: Довольно странный набор возможных типов входных аргументов.
|
||||
# TODO: Для чего сюда принимается float, если он все равно приводится к int?
|
||||
# TODO: Из медиатора сюда приходит статус 0.5, который становится нулем.
|
||||
# Это точно ожидаемое и корректное поведение? Сомнительно...
|
||||
# TODO: if isinstance(msg, float): ...
|
||||
if type(msg) == float or type(msg) == int:
|
||||
self.signal_statusBar.emit(int(msg))
|
||||
else:
|
||||
|
||||
@ -18,6 +18,8 @@ class DataConverter(BaseDataConverter):
|
||||
dataframe[bool_columns] = dataframe[bool_columns].replace({True: 1, False: 0})
|
||||
return dataframe
|
||||
except:
|
||||
# TODO: Обработка исключений!
|
||||
# TODO: Осмысленное сообщение в лог. Без traceback и прочего подобного.
|
||||
return None
|
||||
|
||||
def convert_data(self, files: list[str]) -> None:
|
||||
@ -25,10 +27,12 @@ class DataConverter(BaseDataConverter):
|
||||
dataframes = [pd.read_csv(file) if file != '' else None for file in files]
|
||||
converted_dataframes = list(map(self._replace_bool, dataframes))
|
||||
except:
|
||||
# TODO: Обработка исключений!
|
||||
# Get the traceback object
|
||||
tb = sys.exc_info()[2]
|
||||
tbinfo = traceback.format_tb(tb)[0]
|
||||
pymsg = "Traceback info:\n" + tbinfo + "\nError Info:\n" + str(sys.exc_info()[1])
|
||||
# TODO: Осмысленное сообщение в лог. Без traceback и прочего подобного.
|
||||
logger.error(pymsg)
|
||||
converted_dataframes = [None]
|
||||
finally:
|
||||
|
||||
@ -16,6 +16,7 @@ class Mediator(BaseMediator):
|
||||
data: Union[list[str], list[pd.DataFrame], list[list], list[QWidget]]):
|
||||
|
||||
if issubclass(source.__class__, BaseDirectoryMonitor):
|
||||
# TODO: self._controller.update_status("CSV found! Calculating...") А вот так нельзя написать?
|
||||
self.update_status("CSV found! Calculating...")
|
||||
self._converter.convert_data(data)
|
||||
|
||||
@ -30,12 +31,17 @@ class Mediator(BaseMediator):
|
||||
if issubclass(source.__class__, BasePlotWidget):
|
||||
self.update_status(100)
|
||||
self._controller.send_widgets(data)
|
||||
|
||||
|
||||
def update_settings(self, settings: list[dict]):
|
||||
# TODO: А дописать self.notify не?
|
||||
# if issubclass(source.__class__, BaseController):
|
||||
# self._monitor.update_settings(data)
|
||||
# self._passportFormer.update_settings(data)
|
||||
self._monitor.update_settings(settings)
|
||||
self._passportFormer.update_settings(settings)
|
||||
|
||||
def update_status(self, msg: Union[str, float]) -> None:
|
||||
# TODO: Бессмысленный метод.
|
||||
self._controller.update_status(msg)
|
||||
|
||||
|
||||
|
||||
@ -7,10 +7,11 @@ from utils.base.base import BaseDirectoryMonitor
|
||||
class DirectoryMonitor(BaseDirectoryMonitor):
|
||||
|
||||
def _init_state(self):
|
||||
|
||||
self._files = [
|
||||
os.path.join(self._directory_path, file)
|
||||
for file in os.listdir(self._directory_path)
|
||||
# TODO: FileNotFoundError: [Errno 2] No such file or directory: 'D:/downloads/a22'
|
||||
# при выборе real time mode.
|
||||
if file.lower().endswith('.csv')
|
||||
]
|
||||
|
||||
@ -31,6 +32,9 @@ class DirectoryMonitor(BaseDirectoryMonitor):
|
||||
if not current_files:
|
||||
self._files = []
|
||||
|
||||
# TODO: Почему монитор директории обновляет какие-то настройки, графики, стартует что-то?
|
||||
# Это не должно быть в его зоне ответственности.
|
||||
# TODO: Может стоит сделать какой-то класс, который будет заниматься обновлениями?
|
||||
def update_settings(self, data: list[dict]) -> None:
|
||||
if self.isActive:
|
||||
self.stop()
|
||||
|
||||
@ -7,6 +7,7 @@ from utils.base.base import BasePointPassportFormer, BaseIdealDataBuilder
|
||||
|
||||
|
||||
class idealDataBuilder(BaseIdealDataBuilder):
|
||||
# TODO: Имя класса с большой буквы.
|
||||
def get_closingDF(self) -> pd.DataFrame:
|
||||
return self._get_data(self.Ts['tclose'], self.calcPhaseClose)
|
||||
|
||||
@ -35,10 +36,13 @@ class idealDataBuilder(BaseIdealDataBuilder):
|
||||
class PassportFormer(BasePointPassportFormer):
|
||||
|
||||
def form_passports(self, data: list[pd.DataFrame]) -> list[list[pd.DataFrame, dict, int]]:
|
||||
# TODO: сигнатура метода не соответствует сигнатуре метода класса родителя
|
||||
try:
|
||||
return_data = [self._build_passports_pocket(dataframe) for dataframe in data]
|
||||
except:
|
||||
# TODO: обработка исключений!!!
|
||||
tb = sys.exc_info()[2]
|
||||
# TODO: Нормальные сообщения в лог!
|
||||
tbinfo = traceback.format_tb(tb)[0]
|
||||
pymsg = "Traceback info:\n" + tbinfo + "\nError Info:\n" + str(sys.exc_info()[1])
|
||||
logger.error(pymsg)
|
||||
@ -48,7 +52,8 @@ class PassportFormer(BasePointPassportFormer):
|
||||
|
||||
|
||||
def _build_passports_pocket(self, dataframe: pd.DataFrame) -> list[pd.DataFrame, dict, int]:
|
||||
|
||||
# TODO: Еще раз проверь, что написано в аннотации, и что метод возвращает.
|
||||
# TODO: Практически нечитаемо. Надо оптимизировать и декомпозировать.
|
||||
if dataframe is not None:
|
||||
events, point_quantity = self._filter_events(dataframe["time"], dataframe)
|
||||
if point_quantity == 0:
|
||||
@ -74,16 +79,21 @@ class PassportFormer(BasePointPassportFormer):
|
||||
}
|
||||
params_list = [operator_settings, system_settings]
|
||||
cache_key = self._generate_cache_key(params_list)
|
||||
# TODO: if-else заменить на:
|
||||
# ideal_data = self._ideal_data_cashe.get(cache_key, self._build_ideal_data(idealDataBuilder=idealDataBuilder, params=params_list))
|
||||
# self._ideal_data_cashe[cache_key] = ideal_data
|
||||
if cache_key in self._ideal_data_cashe :
|
||||
ideal_data = self._ideal_data_cashe[cache_key]
|
||||
else:
|
||||
ideal_data = self._build_ideal_data(idealDataBuilder=idealDataBuilder, params=params_list)
|
||||
self._ideal_data_cashe[cache_key] = ideal_data
|
||||
# TODO: point_timeframe, point_events = None, None
|
||||
if events is not None:
|
||||
idx = i+1 if idx_shift else i
|
||||
point_timeframe = [events[self._stages[0]][0][i], events[self._stages[-1]][1][idx]]
|
||||
point_events = {key: [value[0][i], value[1][i]] for key, value in events.items()}
|
||||
else:
|
||||
# TODO: убрать else
|
||||
point_timeframe, point_events = None, None
|
||||
useful_p_data = {"thickness": operator_settings["object_thickness"],
|
||||
"L2": operator_settings["distance_l_2"],
|
||||
|
||||
@ -14,8 +14,18 @@ from gui.reportGui import ReportSettings
|
||||
class MainWindow(BaseMainWindow):
|
||||
def __init__(self,
|
||||
controller: Optional[BaseController] = None) -> None:
|
||||
# TODO: Почему контроллер Optional? Нигде нет обработки случаев, если он будет None: при вызове его методов
|
||||
# будет ошибка.
|
||||
|
||||
# TODO: Касательно передачи экземпляра контроллера в конструктор. Не лучше ли реализовать взаимодействие с ним
|
||||
# через сигналы? Как минимум это позволит исключить странную зависимость интерфейса от контроллера.
|
||||
# Логичнее было бы использовать данный класс в качестве фасада над виджетами, о чем сказано ниже.
|
||||
super().__init__()
|
||||
self._controller = controller
|
||||
# TODO: касательно ВСЕХ методов данного класса, СОЗДАЮЩИХ ВИДЖЕТЫ. Исходя из названия данного класса - это
|
||||
# финальное главное окно приложения: оно не должно создавать виджеты, а уже использовать готовые, поэтому
|
||||
# правильнее будет каждый создаваемый тут виджет вынести в отдельный модуль в отдельный класс, там выполнять
|
||||
# его создание и настройку, а тут уже получать готовый экземпляр виджета и его отображать.
|
||||
self._init_startUI()
|
||||
self._init_dock_widgets()
|
||||
self._init_menu()
|
||||
@ -194,6 +204,11 @@ class MainWindow(BaseMainWindow):
|
||||
if child.widget() is not None:
|
||||
child.widget().deleteLater()
|
||||
|
||||
# TODO: По методам self._init_seekingUI и self._init_raportUI:
|
||||
# 1. Дублирование кода (блок из первых трех строк) - вынести в отдельный метод
|
||||
# 2. Оба метода призваны проинициализировать UI, но в то же время занимаются созданием виджетов и еще что-то
|
||||
# в контроллере дергают: создание виджетов вынести отдельно, контроллеру сообщить о чем-то при помощи
|
||||
# pyqtSignal при нажатии кнопки или выборе пункта меню.
|
||||
def _init_seekingUI(self) -> None:
|
||||
self._clear()
|
||||
self._init_TabWidget()
|
||||
@ -292,6 +307,7 @@ class MainWindow(BaseMainWindow):
|
||||
def _close_tab(self, index:int) -> None:
|
||||
self.tabWidget.removeTab(index)
|
||||
|
||||
# TODO: Зачем разделять эти 2 метода? В self._select_csv путь к файлу получили и контроллеру сигналом отправили.
|
||||
def _open_file(self) -> None:
|
||||
path = self._select_csv()
|
||||
if path is not None:
|
||||
@ -307,7 +323,8 @@ class MainWindow(BaseMainWindow):
|
||||
def _on_tab_changed(self, index):
|
||||
try:
|
||||
tab = self.tabWidget.currentWidget()
|
||||
except:
|
||||
except:
|
||||
# TODO: Очень плохая практика - не указывать конкретный тип исключений.
|
||||
tab = None
|
||||
if tab is not None and self.report_dock.isVisible():
|
||||
reg_items = tab.property("reg_items")
|
||||
@ -316,6 +333,9 @@ class MainWindow(BaseMainWindow):
|
||||
self.repSettings.build(index, reg_items, curve_items, qt_items)
|
||||
|
||||
def _save_plots(self) -> None:
|
||||
# TODO: Нарушение принципа единичной ответственности. Почему ИНТЕРФЕЙС занимается сохранением чего-то?
|
||||
# От него должен только исходить призыв к действию (кнопка или что-то подобное), сама логика же должна
|
||||
# быть в другом месте.
|
||||
filepath, _ = QtWidgets.QFileDialog.getSaveFileName(self, "Save file", "", "Image Files (*.png *.jpeg)")
|
||||
tab = self.tabWidget.currentWidget()
|
||||
if tab is not None:
|
||||
|
||||
@ -13,8 +13,12 @@ import pandas as pd
|
||||
from typing import Optional, Any
|
||||
|
||||
from utils.base.base import BasePlotWidget
|
||||
# TODO: Навести порядок в импортах.
|
||||
|
||||
class ProcessStage():
|
||||
# TODO: Для чего тут наследование от ничего?
|
||||
# TODO: Если я правильно понял сценарий использования этого класса, то правильно это сделать либо отнаследовавшись
|
||||
# от NamedTuple, либо использовать @dataclass.
|
||||
mean_value:int
|
||||
start_index:int
|
||||
finish_index:int
|
||||
@ -157,6 +161,7 @@ class PlotWidget(BasePlotWidget):
|
||||
"""
|
||||
Добавляет QLabel с информацией о производительности.
|
||||
"""
|
||||
# TODO: Почему PlotWidget создает Label? Вынести в другое место.
|
||||
tesla_TWC = round((1 - TWC_time/tesla_time)*100, 2) if tesla_time else 0.0
|
||||
tesla_ideal = round((1 - ideal_time/tesla_time)*100, 2) if tesla_time else 0.0
|
||||
TWC_ideal = round((ideal_time/TWC_time)*100, 2) if TWC_time else 0.0
|
||||
@ -187,6 +192,12 @@ class PlotWidget(BasePlotWidget):
|
||||
|
||||
|
||||
def _build_widget(self, data: list[Any]) -> QWidget:
|
||||
# TODO: Либо передавать в метод 3 аргумента с аннотацией типа каждого, либо передавать
|
||||
# NamedTuple / dataclass.
|
||||
# TODO: Исходя из сигнатуры метода, он создает и возвращает виджет с графиками.
|
||||
# Простыня "result_widget, reg_items, curve_items, qt_items" не похожа на виджет с графиком.
|
||||
# TODO: Данный метод должен содержать только ту логику, которая относится непосредственно к графикам.
|
||||
# Все остальное - вынести. Оставшийся метод декомпозировать - очень трудно вообще понять, что тут происходит.
|
||||
"""
|
||||
Собирает графический виджет для одного набора данных.
|
||||
Параметр `data` предполагается списком: [dataframe, points_pocket, useful_data].
|
||||
@ -353,19 +364,26 @@ class PlotWidget(BasePlotWidget):
|
||||
...
|
||||
]
|
||||
"""
|
||||
# TODO: Про входные аргументы см. выше.
|
||||
try:
|
||||
# TODO: Про инициализацию атрибутов класса где-то уже писал. На всякий случай: она делается в конструкторе.
|
||||
self._datalen = len(data)
|
||||
widgets_datapack = [self._build_widget(data_sample) for self._datastep, data_sample in enumerate(data)]
|
||||
except:
|
||||
# TODO: Добавить конкретные исключения.
|
||||
tb = sys.exc_info()[2]
|
||||
tbinfo = traceback.format_tb(tb)[0]
|
||||
pymsg = "Traceback info:\n" + tbinfo + "\nError Info:\n" + str(sys.exc_info()[1])
|
||||
# TODO: Логи должны быть понятны обычному пользователю. Traceback что-то там - это не информативно и может
|
||||
# быть непонятно, поэтому пишем осознанные сообщения. Если надо, то создаем собственные
|
||||
# классы исключений. (Касается всего проекта - уже не первый раз натыкаюсь на это.)
|
||||
logger.error(pymsg)
|
||||
widgets_datapack = [QLabel(pymsg)]
|
||||
finally:
|
||||
self._mediator.notify(self, widgets_datapack)
|
||||
|
||||
def _update_status(self, widgsteps:int, pointsteps:int, cur_widg:int, cur_point:int):
|
||||
# TODO: if self._datalen: ...
|
||||
if self._datalen != 0:
|
||||
sycle_start = self._datastep/self._datalen*100 + 1
|
||||
period1 = 100/self._datalen
|
||||
@ -377,6 +395,7 @@ class PlotWidget(BasePlotWidget):
|
||||
period3 = period2/pointsteps if pointsteps != 0 else period2
|
||||
|
||||
progress = sycle_start + period2*cur_widg + period3*cur_point
|
||||
# TODO: см. модуль mediator.py
|
||||
self._mediator.update_status(progress)
|
||||
|
||||
|
||||
|
||||
@ -103,7 +103,7 @@ class settingsWindow(QWidget):
|
||||
for i, (_, items) in enumerate(self._data.items()):
|
||||
for j, item in enumerate(items):
|
||||
self._param_table.setItem(i, j, QTableWidgetItem(str(item)))
|
||||
|
||||
# TODO: if isinstance()...
|
||||
if type(item) == int:
|
||||
self._param_table.setItemDelegateForRow(i, int_delegate)
|
||||
elif type(item) == float:
|
||||
|
||||
@ -1,6 +1,8 @@
|
||||
import sys
|
||||
import pyqtgraph as pg
|
||||
from PyQt5 import QtWidgets
|
||||
# TODO: Правило хорошего тона при оформлении импортов: сначала встроенные библиотеки,
|
||||
# через одну пустую строку - дополнительно установленные, и еще через одну пустую строку - собственные импорты.
|
||||
|
||||
from gui.mainGui import MainWindow
|
||||
from controller.monitor import DirectoryMonitor
|
||||
@ -10,6 +12,8 @@ from gui.plotter import PlotWidget
|
||||
from controller.controller import Controller
|
||||
from controller.passportFormer import PassportFormer
|
||||
|
||||
# TODO: Актуализировать requirements.txt!!! Делать это регулярно после добавления новых библиотек!
|
||||
# TODO: Именование модулей: lowercase / snake_case.
|
||||
|
||||
def main():
|
||||
pg.setConfigOptions(useOpenGL=False, antialias=False)
|
||||
|
||||
@ -4,15 +4,21 @@ from typing import Optional
|
||||
import os
|
||||
import numpy as np
|
||||
import pandas as pd
|
||||
|
||||
# TODO: Оформление импортов
|
||||
|
||||
|
||||
class BasePerformanceFactory(ABC):
|
||||
# TODO: Абстракция делается с целью определения интерфейса и только: внутри никаких реализаций быть не должно.
|
||||
# Сначала делаешь абстракцию, затем от нее наследуешься и делаешь нечто базовое, затем конкретную реализацию.
|
||||
# Есть вариант не делать базовое нечто, а сразу переходить к реализации.
|
||||
|
||||
# TODO: Убрать из этого класса все реализации и имплементировать соответствующие методы уже в наследниках.
|
||||
# TODO: Возможно стоит отдельно вынести все касающееся парсинга в отдельный класс (на подумать).
|
||||
|
||||
@abstractmethod
|
||||
def factory_method(self):
|
||||
...
|
||||
|
||||
# TODO: @abstractmethod
|
||||
def job(self):
|
||||
...
|
||||
|
||||
@ -30,17 +36,25 @@ class BasePerformanceFactory(ABC):
|
||||
inside_channel = False
|
||||
channels = 0
|
||||
for line in file:
|
||||
line = line.strip()
|
||||
line = line.strip()
|
||||
# TODO: if line in ("#BEGINCHANNELHEADER", "#BEGINGLOBALHEADER")...
|
||||
if line == '#BEGINCHANNELHEADER' or line == "#BEGINGLOBALHEADER":
|
||||
inside_channel = True
|
||||
|
||||
# TODO: if line in ("#ENDCHANNELHEADER", "#ENDGLOBALHEADER")...
|
||||
elif line == '#ENDCHANNELHEADER' or line == "#ENDGLOBALHEADER":
|
||||
inside_channel = False
|
||||
# TODO: для чего тут pass?
|
||||
pass
|
||||
# TODO: inside_channel = line in ("#BEGINCHANNELHEADER", "#BEGINGLOBALHEADER")
|
||||
# Формирование словаря
|
||||
# TODO: Предыдущие условия связаны были с line, а теперь под elif уже абсолютно другая логика.
|
||||
# Не надо так делать.
|
||||
elif inside_channel:
|
||||
# TODO: нижним подчеркиванием обозначается переменная, которая как бы есть, но не нужна.
|
||||
# Ты же на что-то проверяешь, т.е. используешь. Назови нормально.
|
||||
_, data = line.split(',')
|
||||
match _:
|
||||
# TODO: тут default case не нужен?
|
||||
case '102':
|
||||
head['rob_id'] = data
|
||||
case '200':
|
||||
@ -58,9 +72,11 @@ class BasePerformanceFactory(ABC):
|
||||
case '241':
|
||||
head[ch_name]['multiplyer'] = float(data)
|
||||
head['channels'] = int(channels)
|
||||
# TODO: Метод декомпозировать и переписать.
|
||||
return head, file
|
||||
|
||||
def _r64_parser(self, path: str, head: dict) -> Optional[list[pd.Series, pd.DataFrame]]:
|
||||
# TODO: Метод декомпозировать и переписать.
|
||||
ch = head['channels']
|
||||
keys = list(head.keys())[-ch:]
|
||||
len_timestamps = head['Zeit']['len']
|
||||
@ -80,6 +96,8 @@ class BasePerformanceFactory(ABC):
|
||||
|
||||
|
||||
class BaseProduct(ABC):
|
||||
# TODO: Аналогично BasePerformanceFactory: снова в абстракции куча реализаций.
|
||||
# TODO: Может стоит дать классу название осмысленное, а не из статьи по паттернам? =)
|
||||
def __init__(self):
|
||||
self._signals = []
|
||||
|
||||
@ -117,10 +135,16 @@ class BaseProduct(ABC):
|
||||
|
||||
@abstractmethod
|
||||
def operation(self):
|
||||
# TODO: Аналогично: что за операция? Что она делает? Не понятно.
|
||||
...
|
||||
|
||||
|
||||
class Performance(BasePerformanceFactory):
|
||||
# TODO: где реализация factory_method? Если не нужен, то надо его убрать из интерфейса.
|
||||
# TODO: данный класс, простите, лебедь раком щуку. На все руки мастер. И то создает, и это создает... И принцип
|
||||
# единичной ответственности нарушает, и принцип подстановки...
|
||||
# Если реализуешь паттерн "Фабричный метод", то реализуй его правильно: для каждого порождаемого объекта свой
|
||||
# класс - создатель, классы наследники в полной мере имплементируют методы класса - родителя...
|
||||
|
||||
def robot_method(self) -> BaseProduct:
|
||||
return RobotData()
|
||||
@ -132,10 +156,12 @@ class Performance(BasePerformanceFactory):
|
||||
return CommData()
|
||||
|
||||
def job(self, path:str, TWC_raw:pd.DataFrame) -> list[pd.DataFrame, list[pd.DataFrame], list[pd.DataFrame]]:
|
||||
# TODO: сигнатура метода не соответствует той, которая определена в родительском классе.
|
||||
robot = self.robot_method()
|
||||
TWC = self.TWC_method()
|
||||
comm=self.comm_method()
|
||||
dataframe = self._get_file_data(path)
|
||||
# TODO: operation в родителе определен без входных параметров, используется с ними. Не надо так делать.
|
||||
rob_comm, rob_df = robot.operation(dataframe)
|
||||
TWC_comm, TWC_df = TWC.operation(TWC_raw)
|
||||
comm_df = comm.operation(rob_comm, TWC_comm)
|
||||
@ -156,6 +182,8 @@ class RobotData(BaseProduct):
|
||||
communication_sig = {'sent':events["$OUT3244"]["fall"], 'received':events[""][""]}
|
||||
point_interval = self._form_intervals(start=events["$OUT3012"]["rise"], end=events["$OUT3012"]["fall"])
|
||||
movement_interval = self._form_intervals(start=events["$OUT3244"]["rise"], end=events["$OUT3244"]["fall"])
|
||||
# TODO: возвращаемый результат не соответствует аннотированному.
|
||||
# TODO: зачем возвращать DataFrame? В этом есть какой-то философский смысл?
|
||||
return communication_sig, pd.DataFrame({'in_point':point_interval,
|
||||
'in_move':movement_interval})
|
||||
|
||||
@ -177,6 +205,8 @@ class TWC_Data(BaseProduct):
|
||||
squeeze_interval = self._form_intervals(start=events["Squeeze"]["rise"], end=events["Squeeze"]["fall"])
|
||||
relief_interval = self._form_intervals(start=events["Relief"]["rise"], end=events["Relief"]["fall"])
|
||||
oncoming_interval = self._form_intervals(start=events["Oncoming"]["rise"], end=events["Oncoming"]["fall"])
|
||||
# TODO: возвращаемый результат не соответствует аннотированному.
|
||||
# TODO: зачем возвращать DataFrame? В этом есть какой-то философский смысл?
|
||||
return communication_sig, pd.DataFrame({'in_closing':closing_interval,
|
||||
'in_squeeze':squeeze_interval,
|
||||
'in_relief':relief_interval,
|
||||
|
||||
@ -1,29 +1,52 @@
|
||||
# TODO: Тесты должны лежать в директории tests.
|
||||
|
||||
# TODO: Правило хорошего тона при оформлении импортов: сначала встроенные библиотеки,
|
||||
# через одну пустую строку - дополнительно установленные, и еще через одну пустую строку - собственные импорты.
|
||||
from src.OptAlgorithm.OptAlgorithm import OptAlgorithm
|
||||
from src.utils import read_json
|
||||
|
||||
from matplotlib import pyplot as plt, use
|
||||
|
||||
from numpy import cos, sin, sqrt, cbrt, arcsin, linspace, array
|
||||
|
||||
# TODO: Отступ перед блоком if __name__ == "__main__" - 2 пустых строки.
|
||||
if __name__ == "__main__":
|
||||
|
||||
# TODO: Что означают переменные ниже? Имена переменных должны быть осмысленными, чтобы их смысл был понятен не
|
||||
# не только автору кода.
|
||||
tq = 1
|
||||
ts = linspace(0, tq, 200000)
|
||||
|
||||
operator_params = read_json("params/operator_params.json")
|
||||
system_params = read_json("params/system_params.json")
|
||||
|
||||
operator_params = read_json("../params/operator_params.json")
|
||||
system_params = read_json("../params/system_params.json")
|
||||
|
||||
# TODO: 2 блока кода ниже (с циклами) нарушают принцип DRY (Don't Repeat Yourself):
|
||||
# повторяющийся блок кода следует вынести в отдельную функцию.
|
||||
# Например, в такую:
|
||||
# def foo(params_dict: dict) -> dict:
|
||||
# result_dict = {}
|
||||
# i = 1
|
||||
# for key, value in params_dict.items():
|
||||
# if isinstance(value, list):
|
||||
# result_dict[key] = value[i] if len(value) > i else value[0]
|
||||
# else:
|
||||
# result_dict[key] = value
|
||||
# return result_dict
|
||||
# И потом вызвать ее как-то так:
|
||||
# non_array_operator_params, non_array_system_params = foo(operator_params), foo(system_params)
|
||||
|
||||
non_array_operator_params = {}
|
||||
i = 1
|
||||
for key, value in operator_params.items():
|
||||
# TODO: Не совсем понятен смысл такой проверки. Если хотим проверить принадлежность value к list,
|
||||
# то лучше использовать для этого isinstance.
|
||||
if hasattr(value, "__len__"):
|
||||
# TODO: Можно заменить однострочным выражением: result_dict[key] = value[i] if len(value) > i else value[0]
|
||||
if len(value) > i:
|
||||
non_array_operator_params[key] = value[i]
|
||||
else:
|
||||
non_array_operator_params[key] = value[0]
|
||||
else:
|
||||
non_array_operator_params[key] = value
|
||||
|
||||
|
||||
non_array_system_params = {}
|
||||
for key, value in system_params.items():
|
||||
if hasattr(value, "__len__"):
|
||||
@ -33,7 +56,7 @@ if __name__ == "__main__":
|
||||
non_array_system_params[key] = value[0]
|
||||
else:
|
||||
non_array_system_params[key] = value
|
||||
|
||||
|
||||
|
||||
opt = OptAlgorithm(non_array_operator_params, non_array_system_params)
|
||||
Xs = array([opt.getVar("X1", t) for t in ts])
|
||||
|
||||
@ -11,7 +11,7 @@ class UMLCreator:
|
||||
real_data = []
|
||||
ideal_data = []
|
||||
if not self.theor_mode:
|
||||
|
||||
# TODO: Писать так условия - моветон. Как вариант, можно применить конструкцию match - case.
|
||||
for key, items in self.timings_dict.items():
|
||||
if key == 'closure': ideal_time = self._ideal_time[0]
|
||||
elif key == 'compression': ideal_time = self._ideal_time[1]
|
||||
@ -98,7 +98,7 @@ class UMLCreator:
|
||||
timings_dict: dict,
|
||||
mode: bool,
|
||||
name:str):
|
||||
|
||||
# TODO: Это надо в конструкторе класса делать!
|
||||
self._ideal_time = ideal_time
|
||||
self.bool_dict = bool_dict
|
||||
self.float_dict = float_dict
|
||||
|
||||
@ -1,8 +1,9 @@
|
||||
from plantuml import PlantUML
|
||||
from os.path import abspath
|
||||
|
||||
# TODO: Навести порядок в импортах.
|
||||
|
||||
class Request:
|
||||
# TODO: Имя класса не соответствует тому, что он делает.
|
||||
def __init__(self, server_url: str):
|
||||
self._server_url = server_url
|
||||
|
||||
@ -13,12 +14,15 @@ class Request:
|
||||
self.clear()
|
||||
|
||||
def _startUML(self):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.reqArr.append('@startuml')
|
||||
|
||||
def _endUML(self):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.reqArr.append('@enduml')
|
||||
|
||||
def clear(self):
|
||||
# TODO: Атрибуты класса инициализируются в конструкторе класса.
|
||||
self.timestamps = {}
|
||||
self.reqArr = []
|
||||
self.variables = []
|
||||
@ -27,28 +31,36 @@ class Request:
|
||||
self._startUML()
|
||||
|
||||
def addAnalog(self, name, string):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.variables.append(f'analog "{string}" as {name}')
|
||||
|
||||
def addBinary(self, name, string, style = ''):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
if style: name = name + '<<' + style + '>>'
|
||||
self.variables.append(f'binary "{string}" as {name}')
|
||||
|
||||
def addClock(self, name, string):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.variables.append(f'clock "{string}" as {name}')
|
||||
|
||||
def addConcise(self, name, string):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.variables.append(f'concise "{string}" as {name}')
|
||||
|
||||
def addRobust(self, name, string):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.variables.append(f'robust "{string}" as {name}')
|
||||
|
||||
def appendStr(self, string = ''):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.variables.append(string)
|
||||
|
||||
def addLineStyle(self, name, color = 'green', thicknes = 1):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.lineStyles[name] = [f'LineColor {color}', f'LineThickness {thicknes}']
|
||||
|
||||
def generateSVG(self, name):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self._compileUML(name)
|
||||
filename = abspath(f'{name}.txt')
|
||||
self.server.processes_file(filename, outfile=f'{name}.svg')
|
||||
@ -56,6 +68,7 @@ class Request:
|
||||
#return result
|
||||
|
||||
def setTimestamps(self, name, input):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
for time, state in input:
|
||||
try:
|
||||
self.timestamps[f'@{time}'].append(f'{name} is {state}')
|
||||
@ -63,11 +76,13 @@ class Request:
|
||||
self.timestamps[f'@{time}'] = [f'{name} is {state}']
|
||||
|
||||
def _addTimestamp(self, timecode, vars):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.reqArr.append(timecode)
|
||||
for var in vars:
|
||||
self.reqArr.append(var)
|
||||
|
||||
def _addHeader(self):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self.reqArr.append('<style>')
|
||||
if self.lineStyles:
|
||||
self.reqArr.append('timingDiagram {')
|
||||
@ -80,9 +95,11 @@ class Request:
|
||||
self.reqArr.append('</style>')
|
||||
|
||||
def _addVariables(self):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
for var in self.variables: self.reqArr.append(str(var))
|
||||
|
||||
def _compileUML(self, name):
|
||||
# TODO: Единый стиль именования методов! (snake_case / lowercase).
|
||||
self._addHeader()
|
||||
self._addVariables()
|
||||
|
||||
|
||||
@ -1,5 +1,9 @@
|
||||
from __future__ import annotations
|
||||
# TODO: Почему каталог с базовыми классами лежит внутри каталога с утилитами? Это странно.
|
||||
|
||||
|
||||
from __future__ import annotations
|
||||
# TODO: Правило хорошего тона при оформлении импортов: сначала встроенные библиотеки,
|
||||
# через одну пустую строку - дополнительно установленные, и еще через одну пустую строку - собственные импорты.
|
||||
import os
|
||||
from typing import Optional, Union, Any
|
||||
from cachetools import LRUCache
|
||||
@ -13,12 +17,15 @@ from OptAlgorithm import OptAlgorithm
|
||||
from utils.qt_settings import dark_style
|
||||
|
||||
|
||||
# TODO: Глобальное замечание: Базовые классы создаются для обозначения неких базовых свойств объектов.
|
||||
# Конкретная же реализация возлагается на классы - наследники.
|
||||
|
||||
|
||||
class BaseMediator:
|
||||
def __init__(self,
|
||||
monitor: BaseDirectoryMonitor,
|
||||
converter: BaseDataConverter,
|
||||
# TODO: passport_former
|
||||
passportFormer: BasePointPassportFormer,
|
||||
plot: BasePlotWidget,
|
||||
controller: BaseController):
|
||||
@ -37,12 +44,13 @@ class BaseMediator:
|
||||
source: Union[BaseDirectoryMonitor, BaseDataConverter, BasePointPassportFormer, BasePlotWidget],
|
||||
data: Union[list[str], list[pd.DataFrame], list[list], list[QWidget]]):
|
||||
...
|
||||
# TODO: Отступы между методами класса - одна пустая строка.
|
||||
def update_settings (self, data: list[dict]):
|
||||
...
|
||||
|
||||
def update_status(self, msg: Union[str, float]) -> None:
|
||||
...
|
||||
|
||||
# TODO: Отступы между классами - две пустых строки.
|
||||
class BaseDirectoryMonitor:
|
||||
|
||||
update_timer = QTimer()
|
||||
@ -88,7 +96,8 @@ class BaseDirectoryMonitor:
|
||||
def stop(self):
|
||||
self.isActive = False
|
||||
self.update_timer.stop()
|
||||
|
||||
# TODO: Почему монитор директорий обновляет какие-то настройки, графики и т.д.? Помним про
|
||||
# принцип единичной ответственности.
|
||||
def update_settings(self, data: list[dict]) -> None:
|
||||
...
|
||||
|
||||
@ -96,6 +105,7 @@ class BaseDirectoryMonitor:
|
||||
...
|
||||
|
||||
def force_all_dir(self):
|
||||
# TODO: По сигнатуре метода вообще не понятно, что он должен делать.
|
||||
...
|
||||
|
||||
class BaseDataConverter:
|
||||
@ -230,6 +240,9 @@ class BasePlotWidget:
|
||||
},
|
||||
}
|
||||
def set_style(self, object: Union[QTabWidget, QWidget]) -> None:
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
# TODO: object - зарезервированное слово. Использовать эти слова для именования переменных не правильно.
|
||||
# Если хочется использовать именно такое слово, можно написать "object_".
|
||||
object.setStyleSheet(
|
||||
"""QLabel {
|
||||
color: #ffffff;
|
||||
@ -239,6 +252,8 @@ class BasePlotWidget:
|
||||
}""")
|
||||
|
||||
def _downsample_data(self, x, y, max_points=5000):
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
# TODO: Какой тип данных у 'x'? Какой тип данных у 'y'? Надо добавить аннотации типов.
|
||||
"""
|
||||
Понижает разрешение данных до заданного количества точек для улучшения производительности навигатора.
|
||||
"""
|
||||
@ -273,12 +288,13 @@ class BasePlotWidget:
|
||||
|
||||
# Связываем изменение региона навигатора с обновлением области просмотра основного графика
|
||||
ROI_region.sigRegionChanged.connect(lambda: self._sync_main_plot_with_navigator(main_plot, ROI_region))
|
||||
|
||||
# TODO: Возвращаемый результат не соответствует аннотированному в сигнатуре метода.
|
||||
return navigator, ROI_region
|
||||
|
||||
def _sync_main_plot_with_navigator(self,
|
||||
main_plot: pg.PlotItem,
|
||||
region: pg.LinearRegionItem):
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
"""
|
||||
Синхронизирует область просмотра основного графика с регионом навигатора.
|
||||
"""
|
||||
@ -288,11 +304,14 @@ class BasePlotWidget:
|
||||
main_plot.setXRange(x_min, x_max, padding=0)
|
||||
main_plot.blockSignals(False)
|
||||
|
||||
# TODO: Методы _mirror_shift_data и _shift_data нарушают принцип DRY: дублирование кода.
|
||||
# Сделать ОДИН метод, одним из входных параметров которого будет lambda-функция, которая применяется к dataframe.
|
||||
def _mirror_shift_data(self,
|
||||
valid_str: str,
|
||||
signals: list[dict],
|
||||
dataframe: pd.DataFrame,
|
||||
shift: float) -> pd.DataFrame:
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
keys = dataframe.keys()
|
||||
for signal in signals:
|
||||
if valid_str in signal["name"] and signal["name"] in keys:
|
||||
@ -304,6 +323,7 @@ class BasePlotWidget:
|
||||
signals: list[dict],
|
||||
dataframe: pd.DataFrame,
|
||||
shift: float) -> pd.DataFrame:
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
keys = dataframe.keys()
|
||||
for signal in signals:
|
||||
if valid_str in signal["name"] and signal["name"] in keys:
|
||||
@ -311,6 +331,7 @@ class BasePlotWidget:
|
||||
return dataframe
|
||||
|
||||
def _sync_navigator_with_main(self, main_plot: pg.PlotItem, region:pg.LinearRegionItem):
|
||||
# TODO: Данный метод статичный. Для обозначения подобных методов используется декоратор @staticmethod.
|
||||
"""
|
||||
Синхронизирует регион навигатора с областью просмотра основного графика.
|
||||
"""
|
||||
@ -334,6 +355,8 @@ class BasePlotWidget:
|
||||
|
||||
@opt.setter
|
||||
def opt(self, opt: BaseIdealDataBuilder):
|
||||
# TODO: Атрибуты класса следует сначала определять в конструкторе класса.
|
||||
# TODO: Что такое opt? Optical? Option? Optimal? Optimus Prime?
|
||||
self._opt = opt
|
||||
|
||||
def build(self, data: list[pd.DataFrame]) -> list[QWidget]:
|
||||
@ -349,9 +372,11 @@ class BaseController(QObject):
|
||||
...
|
||||
|
||||
def raport_mode (self) -> None:
|
||||
# TODO: обычно в названиях методов должен присутствовать глагол.
|
||||
...
|
||||
|
||||
def seeking_mode(self) -> None:
|
||||
# TODO: обычно в названиях методов должен присутствовать глагол.
|
||||
...
|
||||
|
||||
def open_file(self, filepath: str) -> None:
|
||||
@ -429,6 +454,7 @@ class BaseMainWindow(QMainWindow):
|
||||
|
||||
@controller.setter
|
||||
def controller(self, controller: BaseController) -> None:
|
||||
# TODO: зачем делать setter для контроллера, если ты его экземпляр передаешь в конструктор?
|
||||
self._controller = controller
|
||||
|
||||
def set_style(self, object: Union[QTabWidget, QWidget, QMainWindow]) -> None:
|
||||
|
||||
@ -9,27 +9,34 @@ class DiagramParser:
|
||||
system_config["Welding_signal"],
|
||||
system_config["Release_signal"],
|
||||
system_config["Oncomming_signal"]]
|
||||
|
||||
# TODO: Переменные в Python именуются в lowercase либо snake_case.
|
||||
self.boolDict = {}
|
||||
self.floatDict = {}
|
||||
self.timingsDict = {}
|
||||
self.theor_mode = False
|
||||
|
||||
def setData(self, path):
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
# TODO: Данный метод устанавливает какой-то режим, читает CSV, наполняет словари некими данными.
|
||||
# Декомпозировать на разные методы, каждый из которых будет решать какую-то свою задачу.
|
||||
if not path:
|
||||
self.theor_mode = True
|
||||
else:
|
||||
self.data = pd.read_csv(path)
|
||||
|
||||
|
||||
# TODO: 2 цикла ниже: дублирование кода. Устранить.
|
||||
for signalName in self.data.columns:
|
||||
# TODO: if isinstance...
|
||||
if type (self.data[signalName].iloc[0]) == np.bool:
|
||||
self.boolDict[signalName] = self._getBoolChanges(signalName)
|
||||
|
||||
for signalName in self.data.columns:
|
||||
# TODO: if isinstance...
|
||||
if type (self.data[signalName].iloc[0]) == np.float64:
|
||||
self.floatDict[signalName] = self._getFloatChanges(signalName)
|
||||
|
||||
for key, items in self.boolDict.items():
|
||||
# TODO: Писать так условия - моветон. Как вариант, можно применить конструкцию match - case.
|
||||
if key == self.signals[0]: name = "closure"
|
||||
elif key == self.signals[1]: name = "compression"
|
||||
elif key == self.signals[2]: name = "welding"
|
||||
@ -46,22 +53,34 @@ class DiagramParser:
|
||||
self.timingsDict[name].append([items[i][0], items[i][0]+0.01])
|
||||
|
||||
def getBoolDict (self) -> dict:
|
||||
# TODO: сделать через декоратор @property:
|
||||
# @property
|
||||
# def bool_dict(self) -> dict:
|
||||
# return self._bool_dict
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
return self.boolDict
|
||||
|
||||
def getFloatDict (self) -> dict:
|
||||
# TODO: сделать через декоратор @property:
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
return self.floatDict
|
||||
|
||||
def getRealTimings(self) -> dict:
|
||||
# TODO: сделать через декоратор @property:
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
return self.timingsDict
|
||||
|
||||
def getMode(self) -> bool:
|
||||
# TODO: сделать через декоратор @property:
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
return self.theor_mode
|
||||
|
||||
def _getBoolChanges(self, signalName) -> list:
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
timeCode = self.data['time']
|
||||
signal_values = self.data[signalName]
|
||||
changes = []
|
||||
|
||||
# TODO: if len(signal_values)...
|
||||
if len(signal_values) > 0:
|
||||
changes.append([float(timeCode.iloc[0]), 'high' if bool(signal_values.iloc[0]) else 'low'])
|
||||
|
||||
@ -73,6 +92,7 @@ class DiagramParser:
|
||||
|
||||
|
||||
def _getFloatChanges(self, signalName) -> list:
|
||||
# TODO: Методы в Python именуются в lowercase либо snake_case.
|
||||
timeCode = self.data['time']
|
||||
signal_values = self.data[signalName]
|
||||
changes = []
|
||||
|
||||
@ -1,3 +1,5 @@
|
||||
# TODO: Было бы логично разместить данный модуль в каталоге gui.
|
||||
|
||||
dark_style = """
|
||||
/*
|
||||
------------------------------------------------------
|
||||
|
||||
Loading…
Reference in New Issue
Block a user