Inheritance and initialisation about sktime-dl HOT 7 CLOSED

Why does 3 fail?

from sktime-dl.

It might be possible to get 3 to work. It failed when just doing a straight swap back to initialising the attributes in the derived class's init:

E       AssertionError: Estimator LSTMRegressor should not set any attribute 
apart from parameters during init. Found attributes ['is_fitted', 'random_state'].

../../anaconda3/envs/sktime_env/lib/python3.7/site-packages/sklearn/utils/estimator_checks.py:2413: AssertionError

• Is there somewhere else appropriate where self.random_state could be initialised?
• Would it be reasonable to initialise self.is_fitted in the build_model function?

Either way, initialising those attributes elsewhere still leaves this error:

../../anaconda3/envs/sktime_env/lib/python3.7/site-packages/sklearn/utils/estimator_checks.py:427: in check_estimator

validate_X(X)

E           ValueError: X must be a pandas.DataFrame, but found:<class 'numpy.ndarray'>

../../anaconda3/envs/sktime_env/lib/python3.7/site-packages/sktime/utils/validation/supervised.py:26: ValueError

I.e. this sklearn check requires the fit function to work with an input numpy array X.
It looks like this point could be rectified easily enough.

from sktime-dl.

Okay, yes, everything else could be initialised in fit, self.is_fitted = True could simply be at the end of the fit method. Random state, when passed to the constructor should simply be stored, i.e. self.random_state = random_state but checked and modified only in fit check_random_state(self.random_state)

from sktime-dl.

Hi @mloning , @james-large , I hope you are both well. I'm in a position where I could pick up this issue now. Are you around to be able to review and merge once done?

from sktime-dl.

Hi @Withington, yes, I'm currently in the process of refactoring sktime, and will be here to review changes

from sktime-dl.

Hello all, good timing, was about to focus in this for a while again. Have another couple of networks to implement, and we can just have a general sitrep, I'll make an issue for it

from sktime-dl.

closing in the tidy up, feel free to raise an new issue after the restructure

from sktime-dl.