[ENH] added test parameters for MatrixProfileClassifier

[MMTrooper]

What does this implement/fix? Explain your changes.

Towards #3429

I decided to add the estimator parameter and set it to the scikit-learn classifier KNeighborsClassifier.

I also added my self as a contributor. Let me know if it was appropriate or I need to make a better implementation.

Any other comments?

Now the test is successful.

PR checklist

For all contributions

• I've added myself to the list of contributors with any new badges I've earned :-)
  How to: add yourself to the all-contributors file in the sktime root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
  See here for full badge reference
• Optionally, I've added myself and possibly others to the CODEOWNERS file - do this if you want to become the owner or maintainer of an estimator you added.
  See here for further details on the algorithm maintainer role.
• The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.

[fkiraly]

These failures look like actual bugs - the next step would be trying to isolate them, and either fix, or report.

[MMTrooper]

@fkiraly I decided to just edit the return statement by removing the if else statement while still making sure that there are at least 2 parameters. It seems everything checks out now. Let me know if this is alright.

[fkiraly]

Thanks!

I notice that the estimator parameter is not covered in get_test_params, we should ensure we have one example where this is set.

n_jobs and random_state are covered elsewhere, these should not be covered by get_test_params.

[MMTrooper]

@fkiraly Ok, I've added the estimator parameter and everything seems alright. Let me know if there's more I need to do or changes I need to make for this pull request.

[fkiraly]

Instead of changing the one existing parameter set, can you please add a new parameter set? Thanks.

[fkiraly]

What I meant, can you please leave the first parameter set as it was? It is used in consistency checks.

[MMTrooper]

@fkiraly Apologies for the back and forth. I think I've implemented the changes correctly now. Thanks for your patience.

[fkiraly]

No worries at all, I haven't been very clear.

Could you kindly return a length-2 list, where the original dictionary is the first element, and the new dictionary is second?

Thanks.

[fkiraly]

Thanks. Could you kindly return a length-2 list, where the original dictionary is the first element, and the new dictionary is second?

Kindly do not include a condition on "results_comparison".

[MMTrooper]

@fkiraly Ok, I have removed the condition. However, I am uncertain if this iteration will pass the checks.

[fkiraly]

However, I am uncertain if this iteration will pass the checks.

May I ask, why? Have you tried locally?

[MMTrooper]

@fkiraly The tests passed locally. I felt it may not pass because the second parameter set did cause a few failures earlier. I am glad that the all checks have been passed now. Once again, I appreciate your patience and willingness to help.

[fkiraly]

I see - the tests were still being skipped in tests._config, I now removed the skips and reactivated the tests.

[MMTrooper]

@fkiraly Ok, it seems that most tests passed except 2 that were skipped. Is there anything else I should do?

[fkiraly]

All good now!

[MMTrooper]

@fkiraly Thank you for accepting my changes! Can I tackle another task on issue #3429 or is there another good first issue that I should look at?

[fkiraly]

#3429 is good to start - you can look at more challenging "good first issues" as well! Up to you.