Skip to content
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] Classifier docs tidy up #52

Merged
merged 51 commits into from Feb 20, 2023
Merged

[DOC] Classifier docs tidy up #52

merged 51 commits into from Feb 20, 2023

Conversation

TonyBagnall
Copy link
Contributor

@TonyBagnall TonyBagnall commented Feb 11, 2023

tidy up some of the docstrings in base classifier and some other minor things:

  1. removing explicit reference to mtype/stype in comments see [DOC] remove references to mtype/machine type scitype/stype #42
  2. refactor names to remove the term boilerplate, which has negative connotations
  3. change attributes set in fit from "parameters" to "attributes"
  4. try formatting code in comments, not done it all to see if it works first
  5. add _estimator_type to attributes
  6. Also removed verbose from EE, not on topic, sorry, but it fails pre-commit
  7. fixes [BUG] B028 No explicit stacklevel keyword argument found.  #67

@TonyBagnall TonyBagnall added documentation Improvements or additions to documentation classification Classification package labels Feb 11, 2023
@TonyBagnall TonyBagnall changed the title Classifier docs [DOC] Classifier docs tidy up Feb 11, 2023
Copy link
Contributor

@patrickzib patrickzib left a comment

Choose a reason for hiding this comment

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

Didn't go through all changes. More of a general question, if we want to replace sktime with scikit-time.

docs/source/api_reference/classification.rst Outdated Show resolved Hide resolved
@TonyBagnall
Copy link
Contributor Author

Didn't go through all changes. More of a general question, if we want to replace sktime with scikit-time.

I guess we do replace as we find it?

@TonyBagnall
Copy link
Contributor Author

regarding notebooks, we need a proper look at the structure, I'm just triaging here

@patrickzib
Copy link
Contributor

I guess, the exclude should be with TEASER, not WEASEL. Even more so, as the failong test is woth TEASER+Catch22

GuzalBulatova
GuzalBulatova previously approved these changes Feb 16, 2023
Copy link
Contributor

@GuzalBulatova GuzalBulatova left a comment

Choose a reason for hiding this comment

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

Thanks a lot, looks very good to me! Have some minor suggestions/questions, none of them is really a blocker.

sktime/classification/base.py Show resolved Hide resolved
sktime/classification/base.py Show resolved Hide resolved
sktime/classification/base.py Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
sktime/classification/distance_based/_elastic_ensemble.py Outdated Show resolved Hide resolved
@TonyBagnall TonyBagnall merged commit 5f7dcc0 into main Feb 20, 2023
@TonyBagnall TonyBagnall deleted the classifier_docs branch February 20, 2023 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
classification Classification package documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] B028 No explicit stacklevel keyword argument found.
4 participants