Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix sporadic forecasting test failure #1280

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

Lopa10ko
Copy link
Collaborator

@Lopa10ko Lopa10ko commented Apr 1, 2024

This is a 🐛 bug fix.

Summary

Context

closes #1279

@Lopa10ko Lopa10ko self-assigned this Apr 1, 2024
Copy link

docu-mentor bot commented Apr 1, 2024

👋 Hi, I'm @Docu-Mentor, an LLM-powered GitHub app
powered by Anyscale Endpoints
that gives you actionable feedback on your writing.

Simply create a new comment in this PR that says:

@Docu-Mentor run

and I will start my analysis. I only look at what you changed
in this PR. If you only want me to look at specific files or folders,
you can specify them like this:

@Docu-Mentor run doc/ README.md

In this example, I'll have a look at all files contained in the "doc/"
folder and the file "README.md". All good? Let's get started!

@Lopa10ko Lopa10ko added the time series related to time series processing label Apr 1, 2024
Copy link
Contributor

github-actions bot commented Apr 1, 2024

All PEP8 errors has been fixed, thanks ❤️

Comment last updated at

@Lopa10ko Lopa10ko requested a review from valer1435 April 1, 2024 16:29
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 79.90%. Comparing base (b711ebe) to head (94df381).

Files Patch % Lines
...entations/models/ts_implementations/statsmodels.py 50.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1280      +/-   ##
==========================================
- Coverage   79.92%   79.90%   -0.03%     
==========================================
  Files         146      146              
  Lines       10049    10065      +16     
==========================================
+ Hits         8032     8042      +10     
- Misses       2017     2023       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@valer1435 valer1435 left a comment

Choose a reason for hiding this comment

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

Запусти 2 раза интеграционники на этой ветке, если все ок -вливай


# check ets params according to statsmodels restrictions
self._check_and_correct_params(endog)
self.log.info(f'Changed the following ETSModel parameters: {self.params.changed_parameters}')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Лучше сделать так:

if self._check_and_correct_params(endog) # need to change return statement
        self.log.info(f'Changed the following ETSModel parameters: {self.params.changed_parameters}')

Чтобы не вводить юзера в заблуждение, что параметры меняются (а они могут и не поменяться)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@valer1435 valer1435 self-requested a review April 5, 2024 07:46
@Lopa10ko Lopa10ko merged commit cd915a8 into master Apr 5, 2024
13 checks passed
@Lopa10ko Lopa10ko deleted the fix-ts-forecasting-integration branch April 5, 2024 17:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
time series related to time series processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Test]: ets, smoothing pipelines causing integration test failure
2 participants