Skip to content

ETSC models#161

Open
leostre wants to merge 47 commits into
mainfrom
teaser
Open

ETSC models#161
leostre wants to merge 47 commits into
mainfrom
teaser

Conversation

@leostre

@leostre leostre commented Jul 16, 2024

Copy link
Copy Markdown
Collaborator

Added Early Time Series Classification models including
Economy K, ECEC, Probability Thresholding, TEASER

@leostre
leostre requested a review from technocreep July 16, 2024 23:51
@pep8speaks

pep8speaks commented Jul 16, 2024

Copy link
Copy Markdown

Thanks for update, @leostre!

Line 18:1: F811 redefinition of unused 'np' from line 5
Line 94:73: W503 line break before binary operator
Line 613:28: W503 line break before binary operator

Line 53:37: F821 undefined name 'anomaly_tsp_in_window'
Line 54:121: E501 line too long (122 > 120 characters)

Line 9:5: F541 f-string is missing placeholders

Line 83:121: E501 line too long (122 > 120 characters)

Line 10:1: F403 'from fedot.core.operations.evaluation.operation_implementations.data_operations.sklearn_transformations import *' used; unable to detect undefined names
Line 11:121: E501 line too long (127 > 120 characters)
Line 105:20: F405 'ScalingImplementation' may be undefined, or defined from star imports: fedot.core.operations.evaluation.operation_implementations.data_operations.sklearn_transformations
Line 106:26: F405 'NormalizationImplementation' may be undefined, or defined from star imports: fedot.core.operations.evaluation.operation_implementations.data_operations.sklearn_transformations
Line 108:30: F405 'ImputationImplementation' may be undefined, or defined from star imports: fedot.core.operations.evaluation.operation_implementations.data_operations.sklearn_transformations
Line 110:23: F405 'KernelPCAImplementation' may be undefined, or defined from star imports: fedot.core.operations.evaluation.operation_implementations.data_operations.sklearn_transformations

Line 73:5: E122 continuation line missing indentation or outdented
Line 81:5: E122 continuation line missing indentation or outdented
Line 93:5: E122 continuation line missing indentation or outdented
Line 100:100: E231 missing whitespace after ','
Line 108:100: E231 missing whitespace after ','

Line 97:9: F541 f-string is missing placeholders

Comment last updated at 2024-07-26 12:35:51 UTC

@Lopa10ko Lopa10ko left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

видна отличная рисерч работа.

я позволил себе накидать пока какие-то очевидные правки.
из общего хочу отметить еще нехватку:

  1. примеров запуска новых моделей и их сравнения
  2. докстрингов к новым моделям (пояснить в google style, что за модель, в чем ее фишка и принцип работы - стоит посмотреть доки sktime)
  3. докстрингов в базовом классе BaseETC и его неочевидный на первый вгляд нейминг (предлагаю EarlyTSClassifier)
  4. юнитов для добавленных моделей

Comment thread fedot_ind/core/metrics/interval_metrics.py Outdated
Comment thread fedot_ind/core/metrics/metrics_implementation.py Outdated
Comment thread fedot_ind/core/models/early_tc/base_early_tc.py Outdated
Comment thread fedot_ind/core/models/early_tc/metrics.py
Comment thread fedot_ind/core/models/nn/network_impl/base_nn_model.py Outdated
Comment thread fedot_ind/core/models/nn/network_impl/mlstm.py Outdated
Comment thread fedot_ind/core/models/quantile/quantile_extractor.py Outdated
Comment thread fedot_ind/core/repository/model_repository.py Outdated
Comment thread tests/unit/core/models/test_teaser.py Outdated
@Lopa10ko

Copy link
Copy Markdown
Collaborator

@leostre nitpick относительно 916f899:
можно тогда опустить указание ветки, он автоматом считает с master, но вообще определенная ревизия нужна была для того, чтобы не прокинуть новые баги/проблемы из более новых версий в мастер-ветке. решение временное, поменяем после следующего релиза федота

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants