Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENH] classification test scenario with three classes and pd-multiindex mtype #6374

Merged
merged 10 commits into from
May 29, 2024

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented May 1, 2024

This adds a classification test scenario with three classes and pd-multiindex mtype.

Currently, only two classes were tested.

Depends on bugfixes newly covered:

@fkiraly fkiraly added module:classification classification module: time series classification enhancement Adding new functionality labels May 1, 2024
@fkiraly fkiraly changed the title [ENH] classification test scenario with three classes [ENH] classification test scenario with three classes and pd-multiindex mtype May 28, 2024
fkiraly added a commit that referenced this pull request May 28, 2024
…d-multiindex` mtype (#6491)

This fixes an unreported bug where `BaseClassifier.fit_predict` and
`fit_predict_proba` would not work for `pd-multiindex` mtype.

The failure is due to `cv.split` being called on a multi-index, which
applies to `iloc` range index instead of the first `loc` index component
of the multi-index.

Test coverage is added through the scenario in PR
#6374 which covers both
`pd-multiindex` as well as the three-class case.

This PR also de-duplicates the `_fit_predict_boilerplate` method of
`BaseClassifier` with `BasePanelMixin`.
@fkiraly fkiraly merged commit 2a29959 into main May 29, 2024
16 of 231 checks passed
@fkiraly fkiraly deleted the classif-three-class branch May 29, 2024 08:43
fkiraly added a commit that referenced this pull request May 29, 2024
…data type (#6377)

This PR fixes #6376 by ensuring the `_get_train_probs` method - with
undocumented contract - accepts `X_train` of any permissible panel input
type.

The fix is adding an input converter to numpy3D, which all currently
implemented instances seem to assume.

This approach should fix the failing `test_classifier_output` test for
the new scenario, without removing functionality (even if private), or
degrading efficiency in a case where the input is numpy already.

Depends on #6374 for testing.
geetu040 pushed a commit to geetu040/sktime that referenced this pull request Jun 4, 2024
…d-multiindex` mtype (sktime#6491)

This fixes an unreported bug where `BaseClassifier.fit_predict` and
`fit_predict_proba` would not work for `pd-multiindex` mtype.

The failure is due to `cv.split` being called on a multi-index, which
applies to `iloc` range index instead of the first `loc` index component
of the multi-index.

Test coverage is added through the scenario in PR
sktime#6374 which covers both
`pd-multiindex` as well as the three-class case.

This PR also de-duplicates the `_fit_predict_boilerplate` method of
`BaseClassifier` with `BasePanelMixin`.
geetu040 pushed a commit to geetu040/sktime that referenced this pull request Jun 4, 2024
…dex` mtype (sktime#6374)

This adds a classification test scenario with three classes and
`pd-multiindex` mtype.

Currently, only two classes were tested.

Depends on bugfixes newly covered:

* sktime#6491
geetu040 pushed a commit to geetu040/sktime that referenced this pull request Jun 4, 2024
…data type (sktime#6377)

This PR fixes sktime#6376 by ensuring the `_get_train_probs` method - with
undocumented contract - accepts `X_train` of any permissible panel input
type.

The fix is adding an input converter to numpy3D, which all currently
implemented instances seem to assume.

This approach should fix the failing `test_classifier_output` test for
the new scenario, without removing functionality (even if private), or
degrading efficiency in a case where the input is numpy already.

Depends on sktime#6374 for testing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding new functionality module:classification classification module: time series classification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants