Skip to content

Conversation

TremaMiguel
Copy link
Contributor

Describe the changes you've made

Major changes include:

  1. TBATS and BATS models containers. link
  2. TBATS and BATS examples in notebook. link

Minor changes include:

  1. On class container initialization call args, tune_args, tune_grid and tune_distributions are called through methods that can be accessed as attributes. link.
  2. Filter UserWarning and ConvergenceWarning in time series testing. link
  3. For blend methods test only a subset of the models, tests take too long. link.
  4. Fix AttributeError: 'float' object has no attribute 'round'. link

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

No new tests needed.

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

Copy link
Member

@Yard1 Yard1 left a comment

Choose a reason for hiding this comment

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

There are a lot of formatting changes, I think you may have different black settings that the rest of us - we'll need to figure that out

def test_blend_model_predict(load_setup, load_models):

ts_experiment = load_setup
ts_models = load_models
ts_weights = [uniform(0, 1) for _ in range(len(load_models))]
ts_models = list(np.random.choice(load_models, 5))
Copy link
Member

Choose a reason for hiding this comment

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

Randomness is a bad idea for any sort of tests. We should provide a predefined list of models.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I agree, maybe the more critical ones, excluding random forest, tbats and bats as they take too long.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds fine to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI these are the ones I'm picking for test_blend_model and test_blend_model_predict

_BLEND_TEST_MODELS = [
    'naive',
    'poly_trend',
    'arima'
    'auto_ets',
    'lr_cds_dt',
    'en_cds_dt',
    'knn_cds_dt',
    'dt_cds_dt',
    'lightgbm_cds_dt'
]

2 baseline models, 2 classical statistical, two regress, 2 linear model based, 1 neighbors based, 1 tree based, 1 gradient boosting based

Copy link
Member

@Yard1 Yard1 left a comment

Choose a reason for hiding this comment

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

It looks like numpy is not imported in pycaret/utils.py, can you fix? Aside from that LGTM

@ngupta23 ngupta23 merged commit 3ab9fe1 into time_series May 12, 2021
@ngupta23 ngupta23 added time_series Topics related to the time series and removed time_series_dev labels Sep 19, 2021
@ngupta23 ngupta23 deleted the tbats_bats_models branch September 25, 2021 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
time_series Topics related to the time series
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants