API improvements #990
API improvements #990
Conversation
MorrisNein
added
in progress
MorrisNein
force-pushed
the
api_improvements
branch
5 times, most recently
from
eb4cf57
to
343ce13
Compare
MorrisNein
force-pushed
the
api_improvements
branch
from
199874a
to
1db41a8
Compare
MorrisNein
force-pushed
the
api_improvements
branch
3 times, most recently
from
8406523
to
51f93bd
Compare
MorrisNein
force-pushed
the
api_improvements
branch
2 times, most recently
from
8b1ec5c
to
51d0950
Compare
|
@andreygetmanov, добавил тесты для определения индексных колонок |
Codecov Report
@@ Coverage Diff @@
## master #990 +/- ##
==========================================
+ Coverage 87.54% 87.80% +0.26%
==========================================
Files 208 208
Lines 13822 13962 +140
==========================================
+ Hits 12100 12260 +160
+ Misses 1722 1702 -20
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| If ``None``, then check the first column's name and use it as index if succeeded | ||
| (see the param ``possible_idx_keywords``).\n | ||
| Set ``False`` to skip the check and rearrange a new integer index. | ||
| possible_idx_keywords: lowercase keys to find. If the first data column contains one of the keys, |
There was a problem hiding this comment.
Не понимаю, а зачем нужен этот парамметр. В каких случаях пользователь будет его заполнять? Как мне кажется, он дублирует функциональность парамметра index_col.
There was a problem hiding this comment.
Вынести во внешний API было идеей @nicl-nno. Основная задумка в том, что при загрузке нескольких файлов пользователь мог бы один раз указать, какие индексы в них используются.
Сейчас, спустя время, думается, что лучше бы пользователь самостоятельно указывал индекс для каждого конкретного файла, чтобы не перегружать внешний интерфейс. Логика параметра не выглядит явной для внешнего пользователя. @nicl-nno, что думаешь?
There was a problem hiding this comment.
Смысл параметра был в том, что когда мы автоматизированно обрабатываем кучу разнородных файлов - то нам удобно указать набор возможных названий индекса. Наверное, можно это объединить с index_col, но давайте уже в следующих PR про это подумает.
| columns_to_drop: Optional[List[Union[str, int]]] = None, | ||
| columns_to_use: Optional[List[Union[str, int]]] = None): | ||
|
|
||
| def define_index_column(candidate_columns: List[str]) -> Optional[str]: |
There was a problem hiding this comment.
А зачем потребовалось делать один метод в другом?
There was a problem hiding this comment.
Исключительно для организации кода. Внутренняя функция не используется нигде больше, она максимально "локальная", сокрыта от внешнего контекста
| def is_column_name_suitable_for_index(column_name: str) -> bool: | ||
| return any(key in column_name.lower() for key in possible_idx_keywords) | ||
|
|
||
| columns_to_drop = copy(columns_to_drop) or [] |
There was a problem hiding this comment.
Чтобы не изменять список, переданный извне входным аргументом
There was a problem hiding this comment.
Так в области функции он же не должен менять переменные, которые находятся вне её? Еще копируешь через copy(), а она копирует только ссылки на объект, находящиеся в оригинале. При этом копируешь в туже самую переменную. columns_to_drop ты никак не изменяешь далее в функции.
There was a problem hiding this comment.
В питоне мутабельные объекты передаются по ссылке. Т.е. списки вполне изменяются функцией.
За внутренние объекты списка мы тут не беспокоимся, это немутабельные строки или инты.
There was a problem hiding this comment.
Да, действительно, columns_to_drop не изменяю, сделал по аналогии с columns_to_use из-за строки
columns_to_use.append(index_col)
| @@ -177,4 +181,5 @@ def data_strategy_selector(features, target, ml_task: Task = None, is_predict: b | |||
| pd.DataFrame: PandasStrategy, | |||
| np.ndarray: NumpyStrategy, | |||
| str: CsvStrategy, | |||
| PathLike: CsvStrategy, | |||
There was a problem hiding this comment.
А может сделать какой-то выбор? Хотелось бы чтобы по пути не только csv умело открывать, но и numpy массивы. Мб выбирать стратегию от формата файла в конце пути?
There was a problem hiding this comment.
Не знаком с форматом сохранения numpy-массивов на диск. Это часто используется?
There was a problem hiding this comment.
Формат файла .npy. В некоторых экспериментах для одной компании, частично данные хранятся в таких файлах
There was a problem hiding this comment.
Под такую фичу хорошо бы отдельный PR сделать
…idx_keywords` argument
- prevent parsing index as date for time series for consistency; - try to define index column even outside "columns_to_use".
MorrisNein
force-pushed
the
api_improvements
branch
from
51d0950
to
f489698
Compare
Closes #906, #873, #874.
Change list:
Allowed
Fedot.fitto get path tofeaturesandtargetviaPathobjects.Wrote explicit docstring for
Fedot.fit.Used generic type
FeaturesTypeinFedot.fitannotation.Unified index column detection during CSV loading.
Fixed time series loading from numpy arrays.