wilsbj

Results 20 comments of wilsbj

Happy to add the test. The test should more generically be that the base classes appear last in the mro. This can be tested across all estimators. Regarding Permute: Other...

I've added the following: - a test for the MRO order - setters for the _steps-like properties on these three subclasses - a fix to that converts the categories returned...

Oh, now I get what you mean about handling them like in `HeterogeneousEnsembleForecaster`. That might be a better approach. But the fallback_forecaster and the transformer would be caught up in...

`FallbackForecaster` does indeed work currently, but it would need to be included in the list of _steps (_HeterogenousMetaEstimator seems to assume that _steps contains an iterable of all estimators). Since...

The MRO test picked up four more classes where the mixin was listed after the base class. I've now reversed the order of inheritance on these four as well.

> We should be careful to check whether there was a good reason for the reversal - we will probably see based on failing tests (or not). `_ReducerMixin` only adds...

>This change is added as the category is used as the key to extract forecasters out of self.forecasters, and to place them in self.forecasters_. > >Without this change, test_non_state_changing_method_contract fails...

Hi @fkiraly. Here is is a summary for where I am at with this PR: - I attempted to solve the initial test failures that were due to _steps not...

>For this PR, may I suggest to single out the TransformSelectForecaster and GroupByForecaster, and handle those in a dedicated PR? Whereas the larger est of other, simpler to deal with...

Let's move the conversation on `GroupbyCategoryForecaster` and `TransformSelectForecaster` to #8142. For the new `test_baseclasses_last_in_mro_order`, should I move this from `TestAllEstimators` (where I have currently put it) to `TestAllObjects`?