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

Added matrix profile using stumpy #471

Merged
merged 6 commits into from Dec 14, 2020

Conversation

utsavcoding
Copy link
Contributor

matrix_profile.py with soft dependency check for stumpy added

Reference Issues/PRs

#450

What does this implement/fix? Explain your changes.

Interface and wrap stumpy.stump in series-to-series transformer.

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

No hard dependency, but stumpy is introduced as soft dependency

PR checklist

For all contributions
  • I've added myself to the list of contributors.
  • Optionally, I've updated sktime's CODEOWNERS to receive notifications about future changes to these files.
  • I've added unit tests and made sure they pass locally.
For new estimators
  • I've added the estimator to the online documentation.
  • I've updated the existing example notebooks or provided a new one to showcase how my estimator works.

Any other comments?

@mloning mloning changed the title Added matrix_profile.py Added matrix profile using stumpy Nov 6, 2020
@mloning
Copy link
Contributor

mloning commented Nov 6, 2020

As a next step, take a look at the CONTRIBUTING.md to find out where you have to add stumpy to our dependency specification files.

@utsavcoding
Copy link
Contributor Author

As a next step, take a look at the CONTRIBUTING.md to find out where you have to add stumpy to our dependency specification files.

I checked out CONTRIBUTING.md, but I think those are relevant to hard dependency. Do I need to add something for the soft dependency?

@mloning
Copy link
Contributor

mloning commented Nov 8, 2020

@utsavcoding in order to get the unit tests to pass you need to add it in build_tools/requirements.txt and in setup.py extra requirements. If you want to add a tutorial notebook later on, you also have to add it in .binder/requirements.txt.

@utsavcoding
Copy link
Contributor Author

@utsavcoding in order to get the unit tests to pass you need to add it in build_tools/requirements.txt and in setup.py extra requirements. If you want to add a tutorial notebook later on, you also have to add it in .binder/requirements.txt.

@utsavcoding utsavcoding reopened this Nov 13, 2020
Copy link
Contributor

@mloning mloning left a comment

Choose a reason for hiding this comment

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

Hi @utsavcoding, in order to get the unit tests to pass you need to add it in build_tools/requirements.txt and in setup.py extra requirements. If you want to add a tutorial notebook later on, you also have to add it in .binder/requirements.txt.

Let me know if you need help with that, I don't see the error that you see at the moment because stumpy isn't installed because it is not specified in the requirements.

@mloning mloning added the interfacing algorithms Interfacing existing algorithms/estimators from third party packages label Nov 15, 2020
@mloning mloning linked an issue Nov 15, 2020 that may be closed by this pull request
@mloning
Copy link
Contributor

mloning commented Nov 24, 2020

FYI we had some CI issues, but they're now fixed on master, so you need to update your branch with the latest changes on master.

@utsavcoding
Copy link
Contributor Author

@mloning I have synced my local master branch with the latest updates. And I have added the dependency in as per mentioned above, but I am still facing a key error issue due to which 2 of the pytest(test_all_estimators.py, test_all_transformers.py) are failing.

@mloning
Copy link
Contributor

mloning commented Dec 7, 2020

FYI your master was still out-of-sync (see my latest commit), I'm going to take a look over the next few days!

matrix_profile.py with soft dependency check for stumpy only
Matrix Profile transformer for dataframe
@utsavcoding
Copy link
Contributor Author

@mloning I have pushed in the changes with default window_length=3 for matrix profile. Can you please review it.

@utsavcoding
Copy link
Contributor Author

@mloning Added configuration for stumpy in check_soft_dependencies.py(failure of azure build in github).

Copy link
Contributor

@mloning mloning left a comment

Choose a reason for hiding this comment

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

Nice work @utsavcoding, all tests pass, everything looks good to me! I'll leave it open a bit longer in case others want to comment before we merge.

@mloning mloning marked this pull request as ready for review December 13, 2020 13:02
@mloning
Copy link
Contributor

mloning commented Dec 14, 2020

Thanks again @utsavcoding - great work!

@mloning mloning merged commit 42a6041 into sktime:master Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interfacing algorithms Interfacing existing algorithms/estimators from third party packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement stumpy series-to-series transformer
2 participants