-
Notifications
You must be signed in to change notification settings - Fork 146
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
[MNT] Tags using enums #2235 #2437
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to
|
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.
This seems a lot clunkier that just strings at first glance, not sure I'm a fan currently.
aeon/clustering/_clustering_tags.py
Outdated
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.
I would not do this in the clustering module, it should be more integrated with the other tags stuff in aeon/utils/tags
.
Hi @MatthewMiddlehurst @chrisholder, I have made the required changes as commented. Since I have shifted the enum_tags.py to utils module it seems to have a circular import issue for the given test case. Test case: Error: I propose to keep it currently in the clustering module or maybe I would need some help to resolve the error. |
I think this issue requires some discussion before progressing, as its not obvious how best to do this and the changes, whilst under the hood, are fairly significant in terms of design |
Reference Issues/PRs
Contributes to #2235.
What does this implement/fix?
This PR replaces string values in
_tags
with their corresponding Enum values in the Clustering module. These changes improve maintainability and reduce errors by standardizing tag values across the module.Key points:
Enum.value
for consistency.Does your contribution introduce a new dependency?
No
Any other comments?
This change represents a significant milestone in transitioning to Enum-based tags. Approval for this approach in the Clustering module will allow for escalation to other modules.
Some examples of usage:
PR checklist
For all contributions