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.

Sign up for GitHub

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

Jump to bottom

[ENH] revert MrSQM patches once upstream compatibility issues are resolved #4396

Open
fkiraly opened this issue Mar 26, 2023 · 1 comment
Open

[ENH] revert MrSQM patches once upstream compatibility issues are resolved #4396

fkiraly opened this issue Mar 26, 2023 · 1 comment
Labels
enhancement Adding new functionality module:classification classification module: time series classification

Comments

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 26, 2023

This is an issue to track that we need to remove the sktime sided patches to MrSQM once the upstream compatibility issues in mrsqm have been resolved.

Patch revert needs to be applied in three places, in the MrSQM classifier:

  • _get_fitted_params override
  • get_test_params need to include a test case with nsfa strictly greater 0
  • warning about nsfa and persistence needs to be removed in docstring

Original MrSQM issue in sktime: #4338
Upstream issue: mlgig/mrsqm#7

FYI @lnthach, @heerme
@fkiraly fkiraly added module:classification classification module: time series classification enhancement Adding new functionality labels Mar 26, 2023
@fkiraly
Copy link
Collaborator Author

fkiraly commented Aug 28, 2023 •
edited
Loading

upstream issue seems resolved mlgig/mrsqm#7 (comment), so will test compliance with newest version.

PR: mlgig/mrsqm#7 (comment)

This was referenced Aug 28, 2023
Merged
Open
fkiraly added a commit that referenced this issue Dec 23, 2023
@fkiraly
[BUG] enable testing MrSQM for persistence in nsfa>0 case after u…
03017ce 
…pstream bugfix (#5171)

Upstream issues re persistence of `MrSQM` have been fixed in version
`0.0.5` of `mrsqm`, see #4396 and
mlgig/mrsqm#7 (comment).

This PR adds back the `nsfa>0` test case and removes the warning from
the docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Adding new functionality module:classification classification module: time series classification
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@fkiraly