[ENH] Add type hints for deep learning regression classes

[saadaltohamy]

Reference Issues/PRs

#1454

What does this implement/fix? Explain your changes.

Added type hints for all deep learning classes in the aeon/regression/deep_learning directory.

Does your contribution introduce a new dependency? If yes, which one?

No

Any other comments?

NaN

PR checklist

For all contributions

• I've added myself to the list of contributors. Alternatively, you can use the @all-contributors bot to do this for you after the PR has been merged.
• The PR title starts with either [ENH], [MNT], [DOC], [BUG], [REF], [DEP] or [GOV] indicating whether the PR topic is related to enhancement, maintenance, documentation, bugs, refactoring, deprecation or governance.

For new estimators and functions

• I've added the estimator/function to the online API documentation.
• (OPTIONAL) I've added myself as a __maintainer__ at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.

For developers with write access

• (OPTIONAL) I've updated aeon's CODEOWNERS to receive notifications about future changes to these files.

[aeon-actions-bot]

Thank you for contributing to aeon

I have added the following labels to this PR based on the title: [ $\color{#FEF1BE}{\textsf{enhancement}}$ ].
I have added the following labels to this PR based on the changes made: [ $\color{#7E0206}{\textsf{regression}}$ ]. Feel free to change these if they do not properly represent the PR.

The Checks tab will show the status of our automated tests. You can click on individual test runs in the tab or "Details" in the panel below to see more information if there is a failure.

If our pre-commit code quality check fails, any trivial fixes will automatically be pushed to your PR unless it is a draft.

Don't hesitate to ask questions on the aeon Slack channel if you have any.

PR CI actions

These checkboxes will add labels to enable/disable CI functionality for this PR. This may not take effect immediately, and a new commit may be required to run the new configuration.

• Run pre-commit checks for all files
• Run mypy typecheck tests
• Run all pytest tests and configurations
• Run all notebook example tests
• Run numba-disabled codecov tests
• Stop automatic pre-commit fixes (always disabled for drafts)
• Disable numba cache loading
• Push an empty commit to re-run CI checks

[saadaltohamy]

All tests passed,
Could you please review? @baraline @MatthewMiddlehurst @hadifawaz1999 @TonyBagnall

[MatthewMiddlehurst]

I like this in principle, but I have no way to verify its correct currently that isn't too time-consuming given some of them are complex. If you could verify these are correct using mypy or similar that would be great.

[saadaltohamy]

I like this in principle, but I have no way to verify its correct currently that isn't too time-consuming given some of them are complex. If you could verify these are correct using mypy or similar that would be great.

@MatthewMiddlehurst
I run mypy for _cnn.py file using:
mypy --follow-imports=skip --ignore-missing-imports aeon\regression\deep_learning\_cnn.py

and I found this error:
error: Argument "callbacks" to "fit" of "Model" has incompatible type "list[ModelCheckpoint]"; expected "list[Callback] | None" [arg-type]
in this code in line 311:

        self.history = self.training_model_.fit(
            X,
            y,
            batch_size=self.batch_size,
            epochs=self.n_epochs,
            verbose=self.verbose,
  --->      callbacks=self.callbacks_,
        )

this is because in this code line 290

        if self.callbacks is None:
            self.callbacks_ = [
                tf.keras.callbacks.ModelCheckpoint(
                    filepath=self.file_path + self.file_name_ + ".keras",
                    monitor="loss",
                    save_best_only=True,
                ),
            ]
        else:
            self.callbacks_ = self._get_model_checkpoint_callback(
                callbacks=self.callbacks,
                file_path=self.file_path,
                file_name=self.file_name_,
            )

If callbacks is None, it will make self.callbacks_ type is: (variable) callbacks_: list[ModelCheckpoint]
But, if it's not None, it will make self.callbacks_ type is: (variable) callbacks_: list[Callback]

Should I fix this?

I can fix it by just modify this:

if self.callbacks is None:
            self.callbacks_: list[Callback] = [                 # add list[Callback] type hint
                tf.keras.callbacks.ModelCheckpoint(
                    filepath=self.file_path + self.file_name_ + ".keras",
                    monitor="loss",
                    save_best_only=True,
                ),
            ]

[MatthewMiddlehurst]

Nah i would ignore that one for now. Could you check the whole module? I am not 100% familiar with mypy but wouldnt --follow-imports=skip --ignore-missing-imports stop it from checking other files which use the class.

[saadaltohamy]

wouldnt --follow-imports=skip --ignore-missing-imports stop it from checking other files which use the class.
@MatthewMiddlehurst

I initially added --ignore-missing-imports to suppress errors related to missing library stubs, such as:

_encoder.py:21: error: Skipping analyzing "sklearn.utils": module is installed, but missing library stubs or py.typed marker  [import-untyped]
_disjoint_cnn.py:20: error: Skipping analyzing "sklearn.utils": module is installed, but missing library stubs or py.typed marker  [import-untyped]

However, after removing --follow-imports=skip, I encountered a significant number of type-checking errors—most of which are unrelated to the scope of my PR.

I’ve categorized these into seven distinct error types and provided explanations along with potential solutions in this Gist:
https://gist.github.com/saadaltohamy/f85d295539420e9045370eb6c924c427

But I don't know should I fix these or just ignore

[MatthewMiddlehurst]

You only have to fix the ones in the regression deep learning files, my main concern is that it is capturing other files using these i.e. testing.

If there are no errors other than the one you posted previously should be fine.

[saadaltohamy]

If there are no errors other than the one you posted previously should be fine.

Okay I have fixed just some other errors I encountered. You can review the last 2 commits if you want.

Should be okay now, no errors in my files
@MatthewMiddlehurst

[MatthewMiddlehurst]

Don't change code without an explanation of why please considering this is a docs PR. This happens for verbose.

Don't add types to variables, not at that point yet 🙂. The test is just to sanity check your changes, though it did not really work as expected.

[MatthewMiddlehurst]

This does not have to be at the top, just the future import i believe

[saadaltohamy]

I think I should import typing at the top, when imported it in if TYPE_CHECKING, I got this error from pre-commit:

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

aeon/regression/deep_learning/_cnn.py:19:4: F821 undefined name 'TYPE_CHECKING'

[MatthewMiddlehurst]

This was a nice experiment but it has gotten a bit overly complex. mypy still does not work very well currently for our codebase it seems especially with complex types. You are welcome to continue trying to get these to fit, but I will note the original issue was for primitives and strings only.

[saadaltohamy]

Don't change code without an explanation of why please considering this is a docs PR. This happens for verbose.

I have changed verbose from verbose: bool = False to verbose: Literal["auto", 0, 1, 2] = 0 because keras documentation suggests that verbose must be a number not bool

If it's not necessary to do that now, I'll change it back to bool

Don't add types to variables, not at that point yet

Okay, no worries, I'll fix that. Thanks for feedback
@MatthewMiddlehurst

[MatthewMiddlehurst]

Interesting to note, maybe worth creating an issue? Out of scope for this PR though yeah

[saadaltohamy]

maybe worth creating an issue

Sure, I'll do that after finishing this PR..

Sorry for those last errors 😅 I've deleted a line by mistake in save_last_model_to_file function in base class. But it's all good now.
Could you review it again? @MatthewMiddlehurst

[saadaltohamy]

@MatthewMiddlehurst Just a reminder if you have time to review it again. it should be good now.