-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added warning for not running setup before other functions #2354
Conversation
@Yard1 @ryanxjhan |
@ngupta23 Agreed. What about implementing it as a private method? |
The best practice here would be to use a decorator instead of editing all the methods |
Hi @ngupta23, I have implemented the changes. Adding warnings in |
Yes that is correct - these methods do not need the setup to be run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Falling test is unrelated to time series.
Also movement of this check to base class has been added as a separate issue for now.
Related Issuse or bug
Info about Issue or bug
Fixes #2338
Describe the changes you've made
Added warning for not running setup before other functions.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Pytest
Describe if there is any unusual behaviour of your code(Write
NA
if there isn't)NA
Checklist:
Screenshots