Skip to content

Add tsfresh into optional dependencies #1246

Merged
merged 10 commits into from Apr 27, 2023
Merged

Add tsfresh into optional dependencies #1246

merged 10 commits into from Apr 27, 2023

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Apr 26, 2023

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Look #1245.

Closing issues

Closes #1245.

@Mr-Geekman Mr-Geekman self-assigned this Apr 26, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Mr-Geekman Mr-Geekman changed the title Add tsfresh into optional dependancies Add tsfresh into optional dependencies Apr 26, 2023
from etna.experimental.classification.feature_extraction.base import BaseTimeSeriesFeatureExtractor
from etna.experimental.classification.utils import padd_single_series

if SETTINGS.classification_required:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we ok with this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't get how it works, if user don't have pyts, what happens when he try to instantiate WEASEL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the same logic with NNs, look at models/base.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworked importing logic. Probably, it will work better and more clear.

@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2023

Codecov Report

Merging #1246 (f87c9fa) into master (40800fa) will decrease coverage by 0.21%.
The diff coverage is 64.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #1246      +/-   ##
==========================================
- Coverage   87.86%   87.65%   -0.21%     
==========================================
  Files         186      175      -11     
  Lines       10627    10322     -305     
==========================================
- Hits         9337     9048     -289     
+ Misses       1290     1274      -16     
Impacted Files Coverage Δ
etna/libs/tsfresh/distribution.py 33.33% <ø> (-50.01%) ⬇️
etna/libs/tsfresh/significance_tests.py 62.31% <36.36%> (+14.49%) ⬆️
etna/libs/tsfresh/relevance.py 74.28% <80.00%> (-3.81%) ⬇️
etna/settings.py 72.59% <80.00%> (-1.29%) ⬇️
etna/libs/tsfresh/defaults.py 100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Mr-Geekman
Copy link
Contributor Author

I'm not really sure that we want to make tsfresh obligatory and replace etna.libs.tsfresh. Because it has few dependencies, that we don't really need. For example, dask[dataframe].

@github-actions
Copy link

github-actions bot commented Apr 26, 2023

@github-actions github-actions bot temporarily deployed to pull request April 26, 2023 08:49 Inactive
Copy link
Collaborator

@alex-hse-repository alex-hse-repository left a comment

Choose a reason for hiding this comment

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

Don't we want to refresh the methods in libs with the code from new version of tsfresh if it is updated?

@github-actions github-actions bot temporarily deployed to pull request April 27, 2023 15:31 Inactive
@alex-hse-repository alex-hse-repository merged commit 5b9783f into master Apr 27, 2023
13 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tsfresh into optional dependancies
3 participants