-
Notifications
You must be signed in to change notification settings - Fork 205
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] Fix double back tick inconsistencies in classification module docstrings #2695
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to
|
- The checkout step in .github/workflows/pr_precommit.yml was missing the fetch-depth property. - By default, actions/checkout@v4 fetches only the latest commit (fetch-depth: 1), which prevents workflows from accessing the full commit history. - This caused issues in detecting changed files properly. - Setting fetch-depth: 0 ensures a full clone, allowing proper diff detection and resolving potential errors.
@@ -30,6 +30,7 @@ jobs: | |||
repository: ${{ github.event.pull_request.head.repo.full_name }} | |||
ref: ${{ github.head_ref }} | |||
token: ${{ steps.app-token.outputs.token }} | |||
fetch-depth: 0 |
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.
Don't make this edit here. See #2723 if you want to help create help solve this issue
Overview: Input ``n`` series length ``m`` with ``d`` dimensions | ||
TDE searches ``k`` parameter values selected using a Gaussian processes | ||
regressor, evaluating each with a LOOCV. It then retains ``s`` |
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.
These are fine as is. This are the notation used in the paper, not the code.
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.
Just noticed "lcoefficients" below. could you fix that also?
@@ -23,7 +23,7 @@ class WEASEL(BaseClassifier): | |||
""" | |||
Word Extraction for Time Series Classification (WEASEL). | |||
|
|||
As described in [1]_. Overview: Input 'n' series length 'm' | |||
As described in [1]_. Overview: Input ``n`` series length ``m`` |
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.
same here
``chi2`` reduces the number of words, keeping those above the ``p_threshold``. | ||
``random`` reduces the number to at most ``max_feature_count``, | ||
by randomly selecting features. | ||
'none' does not apply any feature selection and yields large bag of words. | ||
``none`` does not apply any feature selection and yields large bag of words. |
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.
String parameters should still contain quotation marks even in code style.
@@ -34,7 +34,7 @@ class WEASEL_V2(BaseClassifier): | |||
""" | |||
Word Extraction for Time Series Classification (WEASEL) v2.0. | |||
|
|||
Overview: Input 'n' series length 'm' | |||
Overview: Input ``n`` series length ``m`` |
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.
Again
``chi2_top_k`` reduces the number of words to at most 'max_feature_count', | ||
dropping values based on p-value. | ||
'random' reduces the number to at most 'max_feature_count', by randomly | ||
``random`` reduces the number to at most ``max_feature_count``, by randomly | ||
selecting features. | ||
'none' does not apply any feature selection and yields large bag of words | ||
``none`` does not apply any feature selection and yields large bag of words |
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.
Same as other weasel
@@ -83,7 +83,7 @@ def test_proportion_train_in_param_finding(): | |||
|
|||
|
|||
def test_all_distance_measures(): | |||
"""Test the 'all' option of the distance_measures parameter.""" | |||
"""Test the ``all`` option of the distance_measures parameter.""" |
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.
no need to edit tests
Greetings @MatthewMiddlehurst, thanks for taking time to review my pull request. I've read all the remarks and will make sure to keep them in mind for all my future contributions to aeon. |
Reference Issues/PRs
Fixes #809
What does this implement/fix? Explain your changes.
This pull request fixes backtick inconsistencies in all docstrings of all classes in the classification module.
The preferred approach mentioned by the maintainers was to use double backticks (
). So, I replaced all single quotes (') with double backticks (
) throughout the classification module.Does your contribution introduce a new dependency? If yes, which one?
No, this PR does not introduce any new dependencies.
Any other comments?
This is a documentation-only change, so no functional code modifications were made.
PR checklist
For all contributions
[DOC]
indicating that this is a documentation-related change.