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] add GMMHMM from hmmlearn #3362

Merged
merged 42 commits into from Sep 21, 2022
Merged

[ENH] add GMMHMM from hmmlearn #3362

merged 42 commits into from Sep 21, 2022

Conversation

miraep8
Copy link
Collaborator

@miraep8 miraep8 commented Aug 29, 2022 •
edited by fkiraly

Building on success I have had with wrapping GuassianHMM in #3156 this is a simple PR which wraps GMMHMM from hmmlearn.

I intend this to be a reference as to how some of the other estimators from hmmlearn can also be wrapped and incorporated into sktime.

Contains PR #3156.

@miraep8
start of hmm wrapper
27a8a02
@miraep8
Adding some comments + basic tests
45dfdd6
@miraep8
Move _check_soft_dependencies
f6ce60f
@miraep8
move hmmlearn import from __init__ to _fit
2a3f506
@miraep8
Changing constructor test + import name
1b1ad87
@miraep8
reshape input
453461c
@miraep8
Fix typo + make reshape more specific
d8f1a30
@miraep8
Update hmm_learn.py
e315526
@miraep8
fixing import name
b114ab6
@miraep8
fix so input is np.array
74aa924
@miraep8
Merge branch 'main' into hmmlearn_wrapper
f8e4243
@miraep8
Fixing docstring test
86d0b57
@miraep8
adding basic test case
78e84ba
@miraep8
Whoops, didn't quite fix docstring error
3e9ba5f
@miraep8
removing changes to test_all_estimators
fb49576
@miraep8
replace vars with dir
21c6cfa
@miraep8
removing attrs
dfeca20
@miraep8
small change to restart tests
261bb48
@miraep8
removing small change
48609fa
@miraep8
fixing example and fit
0c9e92a
@miraep8
fixing docstring
5d566f9
@miraep8
catching the return from fit error fix
8dc28d4
@miraep8
Merge branch 'hmmlearn_wrapper' into other_HMM_from_hmmlearn
19f783f
@miraep8
Merge branch 'main' into hmmlearn_wrapper
e9afeb2
@miraep8
Merge branch 'hmmlearn_wrapper' into other_HMM_from_hmmlearn
787bfbd
@miraep8
beginning GMMHMM
f2f7995
@miraep8
adding missing elements to Parameters section
1e252d7
@miraep8
finish adding GMMHMM
e763f26
@miraep8
Copy link
Collaborator Author

miraep8 commented Aug 29, 2022

As a note this PR depends on #3156 being merged as well.

@miraep8 miraep8 mentioned this pull request Aug 29, 2022
Open
3 tasks
@miraep8
Copy link
Collaborator Author

miraep8 commented Sep 1, 2022

Note, tests are failing, but seems to be due to a potentially sporadic failure of ClearSky? Not sure if this is something folks have seen before or not?

fkiraly
fkiraly requested changes Sep 11, 2022
View reviewed changes
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Same comments as to #3156 apply - kindly isolate soft dependencies (see there)

@miraep8 miraep8 mentioned this pull request Sep 12, 2022
Closed
@lmmentel lmmentel added module:annotation enhancement Adding new functionality labels Sep 13, 2022
fkiraly
fkiraly approved these changes Sep 21, 2022
View reviewed changes
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependencies isolated properly now, thanks!

You forgot to re-request a review, methinks.

@fkiraly fkiraly merged commit b9c5fa7 into sktime:main Sep 21, 2022
@lmmentel lmmentel moved this from Under review to Done in Workstream: annotation, segmentation Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fkiraly fkiraly fkiraly approved these changes

@aiwalter aiwalter Awaiting requested review from aiwalter

Assignees

@miraep8 miraep8

Labels
enhancement Adding new functionality module:annotation
Projects
Workstream: annotation, segmentation
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@miraep8 @fkiraly @lmmentel