Code refactoring proposals #128

epogrebnyak · 2016-04-06T10:35:35Z

To be listed below

Fak3 · 2016-04-06T14:27:11Z

Смешение функционала на уровне модулей.

Использование кода делится на два этапа:

парсинг doc-файла в машиночитаемый вид
использование машиночитаемых данных (публикация и всякая манипуляция)

Для упрощения понимания кода эти два этапа лучше максимально разнести по разным модулям(пакетам).
Эти два модуля могут разделять только описания схем машиночитаемых данных. (в каком-нибудь виде)
Сейчас модули(пакеты) reader и getter взаимно импортруют классы и функции из друг друга и очень трудно понять
где между ними граница.

Смешение сущностей в иерархии классов.

class Publisher(Varnames)
Создание подкласса необходимо только в том случае, если объекты подкласса будут использоваться для доступа
к методам базового класса.
Объекты класса Publisher используются для публикации данных, и никогда не используются для доступа ко входным
данным.
Для генерации входных данных в Publisher достаточно создать внутреннюю переменную self.input_data = Varnames()

Избыточная иерархия классов.

class InputDefinition()
class CoreRowSystem(InputDefinition)
class RowSystem(CoreRowSystem)
Эти классы нигде не используются напрямую. Возможно их логику стоит переместить непосредственно в
производный класс CurrentMonthRowSystem

Много микрофункций, состоящих из одной-двух строчек. Лучше заменить их вызовы на их содержимое.

https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/dataframes.py#L259
https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/dataframes.py#L207
https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/dataframes.py#L133
https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/publish.py#L48

Функции которые используются только в одном месте можно заменить на их содержимое.

https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/reader/rs.py#L164
https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/publish.py#L33
https://github.com/epogrebnyak/rosstat-kep-data/blob/master/kep/getter/publish.py#L38

Fak3 · 2016-04-06T14:30:36Z

Упомянутые проблемы с иерархией классов и с функциями встречаются в коде повсеместно, я дал ссылки только на то что удалось найти за несколько часов. Сейчас подобных проблем в коде очень много, и порог вхождения для сторонних разработчиков измеряется днями

epogrebnyak · 2016-04-06T14:32:07Z

Дайте разбивку этапов на которые можно разбить проект, чтобы его
перерабатывать. Можно ли считать, что блока три:

word2csv - преобразование файлов Word в CSV
csv2db - парсинг "машинночитаемых" CSV файлов и запись в базу данных
db_queries - формирование конечных наборов/выборок данных и их экспорт в итоговые CSV и Excel файлы

Fak3 · 2016-04-06T21:10:03Z

Лучше начинать с простых вещей которые помогут в процессе понять код. Пакет reader лучше отложить на последнее место, как самый сложный.

Развернуть микрофункции и функции которые используются только в одном месте.
Возможно стоит вынести из config.py всё что касается пакета reader, и поместить в его собственный отдельный конфиг.
Проработать и описать API пакета getter
Переделать иерархию классов пакета getter
Разработать единую схему обмена машиночитаемыми данными между reader\getter
Переделать иерархию классов пакета reader

epogrebnyak · 2016-04-07T13:04:30Z

Tentative class structure (no todo)

from word2csv import WordDocuments
from csv2db import RowSystem
from db_query import DataFrames

# folder is data folder with current month word files
# import it from config.py

# create raw csv file if it does not exist
WordDocuments(folder).convert_to_csv()

# raw csv file parsing result as generator
flat_data = RowSystem(folder).get_stream()

# get dataframes and save as files 
holder = DataFrames(gen=flat_data)
dfa, dfq, dfm = holder.get_all()
holder.save_as_csv()
holder.save_as_xl()

# example for parsing

csv_text = """Gross domestic product
billion USD
2000    1455    100 110 121 121
rate of growth, %
2000    1,075   0,99    1,10 1,10 1
"""

headers = {"Gross domestic product":"GDP"}
units = {"billion USD":"bln_usd", "rate of growth, %":"rog"}

labelled_rows = [["VAR_X_bln_usd", "2000", "1455",   "300",  "345",  "400", "410"],
                 [    "VAR_X_rog", "2000", "1,075", "0,99", "1,10", "1,10",   "1"]]

flat_data = [("VAR_X_bln_usd", "a", 2000, None, None, 1455),
             ("VAR_X_bln_usd", "q", 2000,    1, None,  300),
             ("VAR_X_bln_usd", "q", 2000,    2, None,  345),
             ("VAR_X_bln_usd", "q", 2000,    3, None,  400),
             ("VAR_X_bln_usd", "q", 2000,    4, None,  410),
             ("VAR_X_rog", "a", 2000, None, None, 1.075),
             ("VAR_X_rog", "q", 2000,    1, None,  0.99),
             ("VAR_X_rog", "q", 2000,    2, None,  1.10),
             ("VAR_X_rog", "q", 2000,    3, None,  1.10),
             ("VAR_X_rog", "q", 2000,    4, None,     1)]

epogrebnyak · 2016-04-07T13:12:50Z

Развернуть микрофункции и функции которые используются только в одном месте.

не приоритетное

Возможно стоит вынести из config.py всё что касается пакета reader, и поместить в его
собственный отдельный конфиг.

под вопросом, может быть

Проработать и описать API пакета getter

API достаточно простой (получить фреймы dfa, dfq, dfm + записать их в + функционал по varnames - названиям и расшифровкам переменных)

Переделать иерархию классов пакета getter

нужно делать

Разработать единую схему обмена машиночитаемыми данными между reader\getter

Можно уточнить, где есть признаки обмена данными между reader\getter, кроме того что getter получает информацию от reader? Varnames() - он берет данные из разных частей?

Переделать иерархию классов пакета reader

можно делать

Fak3 · 2016-04-07T13:33:19Z

API достаточно простой (получить фреймы dfa, dfq, dfm + записать их в + функционал по varnames - названиям и расшифровкам переменных)

API должен исходить из конкретных юзкейсов, которые у меня пока не получается представить. Кто будет потенциальным пользователем API? Какие именно практические цели поможет ему достичь этот проект?

epogrebnyak · 2016-04-07T13:35:35Z

Юзеркейс тут: https://github.com/epogrebnyak/rosstat-kep-data#Как-использовать-эти-данные
Кейс для админа базы данных: https://github.com/epogrebnyak/rosstat-kep-data#Как-обновить-данные

Fak3 · 2016-04-07T14:11:00Z

Юзеркейс тут: https://github.com/epogrebnyak/rosstat-kep-data#Как-использовать-эти-данные

Интересует скорее не "как" использовать, а "зачем" и кому это будет нужно? То есть конкретно вот эти два вопроса:

Кто будет потенциальным пользователем API?
Какие именно практические цели поможет ему достичь этот проект?

Касаемо того как использовать - возникает вопрос зачем нужна sqlite БД, если она в юзкейсе не используется?

epogrebnyak · 2016-04-07T14:13:07Z

Я затрудняюсь кто и зачем скпщать: пользователи экономической статистики,
для построения иллюстраций и моделей. Им нужен обновляемый цсв файл. Как
еще обьяснить?
7 апр. 2016 г. 17:11 пользователь "Evstifeev Roman" <
notifications@github.com> написал:

Юзеркейс тут:
https://github.com/epogrebnyak/rosstat-kep-data#Как-использовать-эти-данные

Интересует скорее не "как" использовать, а "зачем" и кому это будет нужно?
То есть конкретно вот эти два вопроса:

Кто будет потенциальным пользователем API?

Какие именно практические цели поможет ему достичь этот проект?

Касаемо того как использовать - возникает вопрос зачем нужна sqlite БД,
если она в юзкейсе не используется?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#128 (comment)

Fak3 · 2016-04-07T14:27:31Z

для построения иллюстраций и моделей. Им нужен обновляемый цсв файл.

Ок. Имеющийся юзкейс покрывает только сsv файл. Хочется понять для чего тогда нужен класс KEP и БД?

epogrebnyak · 2016-04-07T14:31:00Z

База данных сейчас может бвть не нужна, в итоге она понадобится, чтобы
хранить данные из нескольких источников, а не только рещультат парсинга
последнего csv. Еще может быть нудна для хранения старвх версий данных
(когда в данных за март есть пересмотр данных за январь).

Fak3 · 2016-04-07T14:44:22Z

KEP - это обертка, млжет быть, иожет не быть.

Предазначение этой обертки непонятно - какие преимущества ее использования по сравнению с созданием стандартного dataframe прямо из csv? Это неплохо бы описать в юзкейсе. Текущий юзкейс описывает как пользователь может использовать данные без изучения дополнительных API, в одну строчку кода. Необходимость в каких-то еще обертках в этом случае у меня не получается оправдать:

dfm = pd.read_csv("data_monthly.txt", converters={'time_index':pd.to_datetime}, index_col='time_index')

epogrebnyak · 2016-04-07T15:40:52Z

Дайте подумаю... KEP - действительно тонкая обертка для создания датафреймов, она может быть или может не быть. Наверное он мне дорог как упражнение, если считаете что лишний, можно его в новой версии убрать.

Предазначение этой обертки непонятно - какие преимущества ее использования по сравнению с созданием стандартного dataframe прямо из csv? Это неплохо бы описать в юзкейсе. Текущий юзкейс описывает как пользователь может использовать данные без изучения дополнительных API, в одну строчку кода. Необходимость в каких-то еще обертках в этом случае у меня не получается оправдать:

dfm = pd.read_csv("data_monthly.txt",
converters={'time_index':pd.to_datetime}, index_col='time_index')

epogrebnyak assigned Fak3 Apr 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code refactoring proposals #128

Code refactoring proposals #128

epogrebnyak commented Apr 6, 2016

Fak3 commented Apr 6, 2016

Fak3 commented Apr 6, 2016

epogrebnyak commented Apr 6, 2016

Fak3 commented Apr 6, 2016

epogrebnyak commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Code refactoring proposals #128

Code refactoring proposals #128

Comments

epogrebnyak commented Apr 6, 2016

Fak3 commented Apr 6, 2016

Fak3 commented Apr 6, 2016

epogrebnyak commented Apr 6, 2016

Fak3 commented Apr 6, 2016

epogrebnyak commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016

Fak3 commented Apr 7, 2016

epogrebnyak commented Apr 7, 2016