[BUG] Fix BOSS based classifiers truncating class names to single character length

[erjieyong]

Reference Issues/PRs

This will fix #4090 which prevent predicted classes list from being populated correctly

What does this implement/fix? Explain your changes.

When calling np.zeros, change string datatype to object so that np.zeros will not truncate the string to only the first character.

I've also tested this locally using provided example in the issues as well as running check_estimator(IndividualBOSS). All tests PASSED!

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

No

[fkiraly]

Looks good to me - could you kindly add a test that certifies for the fix?

I.e., your example (as simple as possible, with dummy data) that fails before the fix and runs after?

It should go into the folder sktime.classification.dictionary_based, in a new module test_boss.py

Let me know if you don't want to add that and/or need help with pytest, best starting point is perhaps following the pattern in test_tde (same folder)

[erjieyong]

Sure! Thanks for guiding me through this. Happy to learn new things! I will try to add the unit test and submit again.

…

Message ID: ***@***.***>

[erjieyong]

@fkiraly ,I need your help. How do I run the tests after writing them?

Using test_tde.py as an example, when I run the following code: the verbose output does not seem show that the tests in test_tde has been excuted. (eg. test_tde_train_estimate)

from sktime.utils.estimator_checks import check_estimator
from sktime.classification.dictionary_based._tde import TemporalDictionaryEnsemble
check_estimator(TemporalDictionaryEnsemble)

Output as follows

All tests PASSED!
{'test_clone[TemporalDictionaryEnsemble]': 'PASSED',
 'test_constructor[TemporalDictionaryEnsemble]': 'PASSED',
 'test_create_test_instance[TemporalDictionaryEnsemble]': 'PASSED',
 'test_create_test_instances_and_names[TemporalDictionaryEnsemble]': 'PASSED',
 'test_estimator_tags[TemporalDictionaryEnsemble]': 'PASSED',
 'test_get_params[TemporalDictionaryEnsemble]': 'PASSED',
 'test_has_common_interface[TemporalDictionaryEnsemble]': 'PASSED',
 'test_inheritance[TemporalDictionaryEnsemble]': 'PASSED',
 'test_no_between_test_case_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredict-0]': 'PASSED',
 'test_no_between_test_case_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredict-1]': 'PASSED',
 'test_no_between_test_case_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-0]': 'PASSED',
 'test_no_between_test_case_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-1]': 'PASSED',
 'test_no_cross_test_side_effects_part1[TemporalDictionaryEnsemble]': 'PASSED',
 'test_no_cross_test_side_effects_part2[TemporalDictionaryEnsemble]': 'PASSED',
 'test_repr[TemporalDictionaryEnsemble]': 'PASSED',
 'test_set_params[TemporalDictionaryEnsemble]': 'PASSED',
 'test_set_params_sklearn[TemporalDictionaryEnsemble]': 'PASSED',
 'test_valid_estimator_class_tags[TemporalDictionaryEnsemble]': 'PASSED',
 'test_valid_estimator_tags[TemporalDictionaryEnsemble]': 'PASSED',
 'test_fit_does_not_overwrite_hyper_params[TemporalDictionaryEnsemble-ClassifierFitPredict]': 'PASSED',
 'test_fit_does_not_overwrite_hyper_params[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate]': 'PASSED',
 'test_fit_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_fit_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_fit_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_fit_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_fit_returns_self[TemporalDictionaryEnsemble-ClassifierFitPredict]': 'PASSED',
 'test_fit_returns_self[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate]': 'PASSED',
 'test_fit_updates_state[TemporalDictionaryEnsemble-ClassifierFitPredict]': 'PASSED',
 'test_fit_updates_state[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredict-get_fitted_params]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_methods_have_no_side_effects[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-get_fitted_params]': 'PASSED',
 'test_multiprocessing_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_multiprocessing_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_multiprocessing_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_multiprocessing_idempotent[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredict-get_fitted_params]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_non_state_changing_method_contract[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-get_fitted_params]': 'PASSED',
 'test_persistence_via_pickle[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_persistence_via_pickle[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_persistence_via_pickle[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_persistence_via_pickle[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredict-get_fitted_params]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_raises_not_fitted_error[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-get_fitted_params]': 'PASSED',
 'test_save_estimators_to_file[TemporalDictionaryEnsemble-ClassifierFitPredict-predict]': 'PASSED',
 'test_save_estimators_to_file[TemporalDictionaryEnsemble-ClassifierFitPredict-predict_proba]': 'PASSED',
 'test_save_estimators_to_file[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict]': 'PASSED',
 'test_save_estimators_to_file[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate-predict_proba]': 'PASSED',
 'test_classifier_on_basic_motions[TemporalDictionaryEnsemble]': 'PASSED',
 'test_classifier_on_unit_test_data[TemporalDictionaryEnsemble]': 'PASSED',
 'test_classifier_output[TemporalDictionaryEnsemble-ClassifierFitPredict]': 'PASSED',
 'test_classifier_output[TemporalDictionaryEnsemble-ClassifierFitPredictMultivariate]': 'PASSED',
 'test_handles_single_class[TemporalDictionaryEnsemble]': 'PASSED',
 'test_multivariate_input_exception[TemporalDictionaryEnsemble]': 'PASSED'}

Appreciate your advice. Thanks.

[fkiraly]

check_estimator only runs the tests that come from TestAllEstimators and TestAllClassifiers etc.

To run the test in, say, test_tde or the new test_boss, you need to use pytest directly.

Two common options are:

• running the test from your developer IDE, here's the tutorial for visual studio code: https://code.visualstudio.com/docs/python/testing
• running pytest via the command line, specifying the module, see here: https://stackoverflow.com/questions/36456920/is-there-a-way-to-specify-which-pytest-tests-to-run-from-a-file

[fkiraly]

Thanks, looks great!
Will merge once tests pass.

One tipp, but not a blocker - the tests are a bit repetitive, and they could be made much less copy-paste by using pytest mark.parametrize (on pairs of the expected y_pred.dtype, and the new_class dict values).

Feel free to try changing that (you can do it in this PR or a separate one once this is merged), as said it´s not necessary for this to go in imo.

[fkiraly]

Ah, but linting is failing now :-)
Here´s the guide: https://www.sktime.org/en/stable/developer_guide/coding_standards.html

[fkiraly]

just fixed the linting so we can potentially put this fix in the release

[erjieyong]

I see there's another issue. Let me clear it on my end first. I have actually installed Black locally, but I guess it didn't catch the particular portion which failed. Will try and install pre-commit

[fkiraly]

I see there's another issue. Let me clear it on my end first.

Yes, it wants you to write docstrings for the tests. Minimal ones have one line, better ones have an explanation of what precisely the test success and fail conditions are.

[erjieyong]

@fkiraly, I've managed to install pre-commit locally already and it has passed all the test. Please review again. Thank you

[fkiraly]

linting passes now! 🎉

[erjieyong]

Thanks a lot for your guidance. I'll work on improving the tests soon!