-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: blending compatible with sktime 0.7.0 #1469
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
Conversation
pycaret/internal/ensemble.py
Outdated
@property | ||
def _set_weights_none(self): | ||
self.weights = None | ||
|
||
@property | ||
def _set_weights_uniform(self): | ||
self.weights = np.ones(len(self.forecasters)) |
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.
Shouldn't those be methods? Is this how sktime does it?
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.
Sktime original implementation doesn't handle voting
method, here I should be changing these to appropiate setter for @property
@property
def weights(self):
return self.weights
@weights.setter
def weights(self, value):
self.weights = value
and then call
elif self.method in self._not_required_weights and self.weights:
warnings.warn(
"Unused 'weights' argument. When method='mean' or method='median', 'weights' argument is not provided. Setting weights to `None`"
)
self.weights = None
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.
@Yard1 I think we should open a PR to include voting
in sktime original implementation, what are your thoughts on this ?
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.
That's a great idea @TremaMiguel
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.
LGTM!
Describe the changes you've made
Reimplement EnsembleForecaster with sktime>=0.7.0
Type of change
Please delete options that are not relevant.
Checklist: