-
Notifications
You must be signed in to change notification settings - Fork 86
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
Ability chosen categorical features while transform data to InputData #1318
Conversation
All PEP8 errors has been fixed, thanks ❤️ Comment last updated at |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1318 +/- ##
==========================================
- Coverage 80.16% 80.04% -0.12%
==========================================
Files 146 146
Lines 10278 10307 +29
==========================================
+ Hits 8239 8250 +11
- Misses 2039 2057 +18 ☔ View full report in Codecov by Sentry. |
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 think it would still be nice to add an example and/or tests so that this useful feature is not lost.
fedot/core/data/data.py
Outdated
if isinstance(categorical_idx[0], str): | ||
raise | ||
else: | ||
categorical_idx = [i for i, column in enumerate(features_names) if column in set(categorical_idx)] | ||
|
||
if isinstance(categorical_idx, list): | ||
categorical_idx = np.array(categorical_idx) | ||
|
||
categorical_features = features[:, categorical_idx].to_numpy() | ||
|
||
data = InputData( | ||
idx=idx, | ||
features=features, | ||
target=target, | ||
task=task, | ||
data_type=data_type, | ||
features_names=features_names, | ||
categorical_idx=categorical_idx, | ||
categorical_features=categorical_features | ||
) |
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.
if isinstance(categorical_idx[0], str): | |
raise | |
else: | |
categorical_idx = [i for i, column in enumerate(features_names) if column in set(categorical_idx)] | |
if isinstance(categorical_idx, list): | |
categorical_idx = np.array(categorical_idx) | |
categorical_features = features[:, categorical_idx].to_numpy() | |
data = InputData( | |
idx=idx, | |
features=features, | |
target=target, | |
task=task, | |
data_type=data_type, | |
features_names=features_names, | |
categorical_idx=categorical_idx, | |
categorical_features=categorical_features | |
) | |
if isinstance(categorical_idx[0], str): | |
raise | |
categorical_idx = [i for i, column in enumerate(features_names) if column in set(categorical_idx)] | |
if len(categorical_idx) > 0: | |
categorical_idx = np.array(categorical_idx) | |
categorical_features = features[:, categorical_idx].to_numpy() | |
else: | |
categorical_idx = None | |
data = InputData( | |
idx=idx, | |
features=features, | |
target=target, | |
task=task, | |
data_type=data_type, | |
features_names=features_names, | |
categorical_idx=categorical_idx, | |
categorical_features=categorical_features | |
) |
nit: кажется, ненужная проверка на список для categorical_idx
. лучше проверить на всякий случай то, что вообще эти списки создаются (в противном случае передать в InputData
значение None
)
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.
можно, наверное, эту логику выделения np.array-ев с проверкой на то, что categorical_idx
и categorical_features
адекватно сформировались, вынести в отдельную приватную функцию
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.
@Lopa10ko Тут проверка в том, что категориальные индексы можно было бы прописывать как через np.ndarray
так и через list
. Проверка, чтобы list перевести в np.ndarray
.
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.
понял, но в текущем варианте categorical_idx
в любом случае будет списком, если я верно понимаю.
вот не вошли в первое ветвление с проверкой на isinstance(categorical_idx[0], str)
не прокинули ошибку, потом ушли в генератор и получили список в любом случае
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.
@Lopa10ko упустил это место во время работы над ним. Я там собирался реализовать то, что если пользователь передает названия колонок, а не индексы и при этом название признаков забыл внести, то как раз вылезает ошибка. Если он указал, то мы преобразуем categorical_idx в индексы и в categorical_features кладем категориальные данные.
Иногда просто легче написать признаки названием, чем высчитывать их индексы
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.
fedot/core/data/data.py
Outdated
categorical_features = None | ||
if categorical_idx is not None: | ||
if isinstance(categorical_idx[0], str): | ||
raise |
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.
хочется получать осмысленное сообщение об ошибке, а не просто raise
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.
@Lopa10ko Да, я хотел написать, но отложил это
fedot/core/data/data.py
Outdated
categorical_features = None | ||
if categorical_idx is not None: | ||
if isinstance(categorical_idx[0], str) and features_names is None: | ||
raise | ||
else: | ||
categorical_idx = [i for i, column in enumerate(features_names) if column in set(categorical_idx)] | ||
|
||
if isinstance(categorical_idx, list): | ||
categorical_idx = np.array(categorical_idx) | ||
|
||
categorical_features = features_array[:, categorical_idx] |
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.
вообще, на самом деле действительно, проще будет просто перенести это в отдельную функцию с валидацией сформированных списков
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.
@Lopa10ko Не совсем понял твое предложение
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.
например, сделать функцию, которая бы делала дополнительную валидацию: если теперь можно передавать categorical_idx
и ответственность ложится на пользователя, нужно проверить, что categorical_features
получаются при этом не пустые (в противном случае передать None)
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.
Зачем проверять? Если categorical_idx = None
, то в ходе предобработки попробует обнаружить категориальные данные. Если categorical_idx = np.array([])
, то категориальных данных нет и categorical_features = np.array([])
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.
Также categorical_features
это категориальные данные, которые не были подвержены кодированию в ходе предобработки и используются в бустинговых моделях
Да, обязательно добавлю тесты, а с примерами не уверен |
кажется, если и добавлять такую возможность, то только для метода в конструктор для хочется посмотреть на такое решение в действии, чтобы было понятно, зачем его прикручивать -- дальше поревьювим |
Да мне кажется норм и в InputData, не так часто нужна такая возможность. |
Ну в принципе ладно, если нужно уточнения в данных, то могут изначально открыть их через все эти методы с указанием категориальных данных. Тогда отставлю и добавлю только тесты |
Migrate to PR #1320 |
This is a 🙋 feature or enhancement.
Summary
Added the ability to specify categorical features when converting data to
InputData
using common methods. During preprocessing, if this information is provided, the specified categorical features are identified based on this input, thereby avoiding the use of an automated and potentially naive method for detecting categorical features. This enhancement is particularly useful as it allows for avoid encoding of " recognized by fault" categorical data with one-hot encoding (OHE) method, which can otherwise consume a significant amount of memory.