Skip to content

Add auto base and auto abstract #1114

Merged
merged 16 commits into from
Feb 22, 2023

Conversation

GooseIt
Copy link
Contributor

@GooseIt GooseIt commented Feb 14, 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

See issue#1023

Closing issues

closes #1023

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Merging #1114 (37f2106) into master (cdf1ee3) will increase coverage by 18.66%.
The diff coverage is 85.71%.

❗ Current head 37f2106 differs from pull request most recent head 881f38e. Consider uploading reports for the commit 881f38e to get more accurate results

📣 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    #1114       +/-   ##
===========================================
+ Coverage   68.39%   87.06%   +18.66%     
===========================================
  Files         170      170               
  Lines        9335     9347       +12     
===========================================
+ Hits         6385     8138     +1753     
+ Misses       2950     1209     -1741     
Impacted Files Coverage Δ
etna/auto/auto.py 90.74% <85.71%> (+48.21%) ⬆️
etna/models/nn/rnn.py 100.00% <0.00%> (ø)
etna/models/sarimax.py 95.00% <0.00%> (+0.71%) ⬆️
etna/models/prophet.py 99.13% <0.00%> (+0.86%) ⬆️
etna/models/holt_winters.py 98.85% <0.00%> (+1.14%) ⬆️
etna/models/catboost.py 100.00% <0.00%> (+1.21%) ⬆️
etna/transforms/math/add_constant.py 97.95% <0.00%> (+2.04%) ⬆️
etna/analysis/outliers/density_outliers.py 97.95% <0.00%> (+2.04%) ⬆️
etna/models/tbats.py 95.00% <0.00%> (+3.33%) ⬆️
etna/metrics/base.py 93.67% <0.00%> (+3.79%) ⬆️
... and 61 more

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

etna/auto/auto.py Outdated Show resolved Hide resolved
etna/auto/auto.py Outdated Show resolved Hide resolved
etna/auto/auto.py Outdated Show resolved Hide resolved
etna/auto/auto.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Mr-Geekman Mr-Geekman left a comment

Choose a reason for hiding this comment

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

Look at comments above.

etna/auto/auto.py Outdated Show resolved Hide resolved
etna/auto/auto.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Mr-Geekman Mr-Geekman left a comment

Choose a reason for hiding this comment

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

Look at comments above.

@GooseIt
Copy link
Contributor Author

GooseIt commented Feb 21, 2023

I've moved most of __init__ from Auto to AutoBase on the basis that every field initialized there except pool will be shared between Auto and Tune classes with same functionality.

etna/auto/auto.py Outdated Show resolved Hide resolved
@Mr-Geekman
Copy link
Contributor

What about refactoring test_auto?

@GooseIt
Copy link
Contributor Author

GooseIt commented Feb 21, 2023

What about refactoring test_auto?

As classes AutoBase and AutoAbstract both have unimplemented abstractmethods (namely fit), I see no tests that can be written to evaluate their performance (these classes cannot even be instantiated).
There is no need to write any additional tests for Auto neither, as there is no new functional added.

If I am wrong, what the refactored tests should check?

@Mr-Geekman
Copy link
Contributor

We have methods summary and top_k in AutoBase. We should check them. In test_auto we have tests test_summary, test_summary, we should change them to work with AutoBase.

You doesn't have to instantiate classes, tests are written using mocks.

@Mr-Geekman Mr-Geekman merged commit 67dfebc into tinkoff-ai:master Feb 22, 2023
@GooseIt GooseIt deleted the add_auto_base_and_auto_abstract branch February 22, 2023 21:23
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.

class AutoBase and AutoAbstract
3 participants