-
Notifications
You must be signed in to change notification settings - Fork 181
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
[DOC] Added Type Hints for _dilated_shapelet_transform #1949
Conversation
Thank you for contributing to
|
Hi, I can take a look at this in a bit if you are still having issues. Feel free to @ me with any questions. @baraline left a comment on your previous PR so im sure he would be open to questions as well |
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.
Some changes required IMO, @baraline might have more for the later function hints
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
@baraline Is there any reason for the tests to be canceled, they were all successful when I committed but got canceled when I updated the branch. |
I don't think that should be happening, let me check |
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.
Re-runned the test, maybe something went wrong with the CI pipeline ? Some small changes and we should be good to go !
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
Some changes were made to the file you are working with on #1959, which added two parameters to the |
Are there any changes that I am supposed to make? |
Yes, could you should first merge the main branch into this and had the type hints for the new parameters of the functions I listed ? |
Added the type hints after merging the main branch. Is there anything else to do? |
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
Outdated
Show resolved
Hide resolved
Just need to resolve the remaining conversation, and we'll be good to go I think ! 😄 |
aeon/transformations/collection/shapelet_based/_dilated_shapelet_transform.py
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.
Looking good to me! Thanks for your patience! 😄
Thanks a lot for the help! |
Changes have been adressed
What does this implement/fix? Explain your changes.
I have added type hints for function variables and return values as described in issue.
#1910