Skip to content

[ENH] Adds RED CoMETS classifier #779

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

Merged
merged 43 commits into from
Oct 12, 2023
Merged

[ENH] Adds RED CoMETS classifier #779

merged 43 commits into from
Oct 12, 2023

Conversation

zy18811
Copy link
Contributor

@zy18811 zy18811 commented Sep 29, 2023

PR adding RED CoMETS classifier.

Not sure what algorithm type it falls under? So have put into hybrid for now.

Adds a soft dependency to imbalanced-learn.

TODO

  • Speed up SAX transforms using parallelisation
  • Fix some minor formatting issues
  • Add tests
  • Add examples in docstrings + update docs/api_reference
  • get_test_params()

PR checklist

  • I've added myself to the list of contributors.
  • Optionally, I've updated aeon's CODEOWNERS to receive notifications about future changes to these files.
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.
  • For new estimators, I've added the estimator to the online documentation.

@TonyBagnall
Copy link
Contributor

great, thanks Luca. The categories are necessarily fluid, but I would probably put it in dictionary based?

@TonyBagnall
Copy link
Contributor

also, would you like to join our slack @zy18811 https://join.slack.com/t/aeon-toolkit/shared_invite/zt-2466yn6zm-5fV0EW8JjSVUoe2hzJjRMw

@TonyBagnall
Copy link
Contributor

could you add some testing code for this? You can use skip test for the soft dependency, for example, someting like this before each test function

@pytest.mark.skipif(
    not _check_soft_dependencies("imblearn", severity="none"),
    reason="skip test if required soft dependency esig not available",
)

@TonyBagnall TonyBagnall added classification Classification package implementing algorithms Implementing new algorithms/estimators labels Sep 29, 2023
@MatthewMiddlehurst MatthewMiddlehurst added the enhancement New feature, improvement request or other non-bug code enhancement label Sep 30, 2023
@zy18811 zy18811 marked this pull request as ready for review October 4, 2023 10:17
TonyBagnall
TonyBagnall previously approved these changes Oct 7, 2023
Copy link
Contributor

@TonyBagnall TonyBagnall left a comment

Choose a reason for hiding this comment

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

couple of very minor docstring comments, but otherwise this looks great, thank you for the contribution

@TonyBagnall
Copy link
Contributor

this test failure is nothing to do with your PR, just a weird tensorflow thing

@zy18811
Copy link
Contributor Author

zy18811 commented Oct 9, 2023

Think I've addressed the two docstring issues :) 🚀

Copy link
Contributor

@TonyBagnall TonyBagnall left a comment

Choose a reason for hiding this comment

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

one more tiny thing on the soft dependencies and I think its ready, great job

@zy18811 zy18811 requested a review from TonyBagnall October 10, 2023 17:54
TonyBagnall
TonyBagnall previously approved these changes Oct 11, 2023
Copy link
Contributor

@TonyBagnall TonyBagnall left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@MatthewMiddlehurst MatthewMiddlehurst left a comment

Choose a reason for hiding this comment

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

Hi Luca, thanks for the contribution. Great work.

One minor change to the all-contributors file would be good, I can monitor and merge after that is complete unless anyone else wants to review and request changes.

@MatthewMiddlehurst MatthewMiddlehurst dismissed stale reviews from TonyBagnall and themself via df51a5f October 11, 2023 13:46
@MatthewMiddlehurst MatthewMiddlehurst merged commit a2cc239 into aeon-toolkit:main Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
classification Classification package enhancement New feature, improvement request or other non-bug code enhancement implementing algorithms Implementing new algorithms/estimators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants