Skip to content
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

CrostonClassic required argument serving no purpose #491

Closed
nickto opened this issue May 19, 2023 · 1 comment · Fixed by #764
Closed

CrostonClassic required argument serving no purpose #491

nickto opened this issue May 19, 2023 · 1 comment · Fixed by #764
Assignees

Comments

@nickto
Copy link
Contributor

nickto commented May 19, 2023

CrostonClassic has a required argument level:

def predict_in_sample(self, level):

However,

  1. The method is not implemented, so it does not make sense to make it required.
  2. None of the other models have level as a required argument in predict_in_sample() method.

Hence, it seems like it should be either

def predict_in_sample(self, level: Optional[Tuple[int]] = None):

to be consistent with other methods that use level argument.

Or

def predict_in_sample(self):

to be consistent with other not implemented methods.

@nickto nickto changed the title CrostonClassic requiredargument serving no purpose CrostonClassic required argument serving no purpose May 19, 2023
@kvnkho kvnkho self-assigned this Jul 14, 2023
@kvnkho
Copy link
Collaborator

kvnkho commented Aug 1, 2023

This should be def predict_in_sample(self, level: Optional[Tuple[int]] = None):.

@jaymanalastas, would you like to fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants