-
Notifications
You must be signed in to change notification settings - Fork 122
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] Implement Proximity Tree classifier using aeon distances #1566
[ENH] Implement Proximity Tree classifier using aeon distances #1566
Conversation
Thank you for contributing to
|
this is good. From our end @MatthewMiddlehurst we should set up a pipeline for running experiments to check equivalence to the Java results, as that was the heart of the problem last time. No rush of course, project has not started yet :) |
I have completed the framework for Proximity Tree along with it's API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments on “surface” content, I'll have a proper look at the algorithmic correctness a bit later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for the most part, left a few comments. We may revisit this later down the line if there are issues, but appears to be fine for now.
I would add this to the API documentation in docs/
in this PR.
There are some if
statements here which are not covered by testing. Not going to block this PR because of them, but if you find the time and can figure out how to get to them reasonably it would be nice. https://app.codecov.io/gh/aeon-toolkit/aeon/pull/1566/blob/aeon/classification/distance_based/_proximity_tree.py
Try merging the main branch for the |
I have merged the main branch, but it is still failing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non blocking changes from me, looks good
aeon/classification/distance_based/tests/test_proximity_tree.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks
Related to: #159
We implemented the
ProximityTree
class using aeon distances.This will be used to create the Proximity Forest class.
ProximityTree
class