Skip to content

[DOC] Issue #14312 - Ensure All Attributes are Documented: OAS: location_ added #17163

Merged
thomasjpfan merged 4 commits intoscikit-learn:masterfrom
hhnnhh:mydoc
May 9, 2020
Merged

[DOC] Issue #14312 - Ensure All Attributes are Documented: OAS: location_ added #17163
thomasjpfan merged 4 commits intoscikit-learn:masterfrom
hhnnhh:mydoc

Conversation

@hhnnhh
Copy link
Copy Markdown
Contributor

@hhnnhh hhnnhh commented May 8, 2020

Reference Issues/PRs

Issue #14312

What does this implement/fix? Explain your changes.

This adds documentation for an attribute of the OAS function and reorders attributes of ShrunkCovariance and LedoitWolf in alphabetical order.

  • OAS: location_ attribute added (covariance_,location_,precision_)
  • ShrunkCovariance: attributes now in alphabetical order (covariance_,location_,precision_)
  • LedoitWolf: attributes now in alphabetical order (covariance_,location_,precision_,shrinkage_)

Any other comments?

ShrunkCovariance: --> shrinkage seems to be a false positive, as it is not an attribute but a parameter for ShrunkCovariance. Hence, shrinkage wasn't added to attributes.

hhnnhh added 3 commits May 8, 2020 11:49
ShrunkCovariance: attributes now in alphabetical order (covariance_,location_,precision_)
--> shrinkage not an attribute, but a parameter. False positive?

LeDoItWolf: attributes now in alphabetical order (covariance_,location_,precision_,shrinkage_)
ShrunkCovariance: attributes now in alphabetical order (covariance_,location_,precision_)
--> shrinkage not an attribute, but a parameter. False positive?

LeDoItWolf: attributes now in alphabetical order (covariance_,location_,precision_,shrinkage_)
…ydoc

# Conflicts:
#	sklearn/covariance/_shrunk_covariance.py
@thomasjpfan
Copy link
Copy Markdown
Member

Is there a discussion where it was decided that attributes should be in alphabetical order?

@TomDLT
Copy link
Copy Markdown
Member

TomDLT commented May 8, 2020

#14312 (comment)

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

The 'OAS' can be removed here:

'NuSVR', 'OAS', 'OneClassSVM', 'OrthogonalMatchingPursuit',

@hhnnhh hhnnhh requested a review from thomasjpfan May 9, 2020 15:06
Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @hhnnhh !

LGTM

@thomasjpfan thomasjpfan merged commit c36c104 into scikit-learn:master May 9, 2020
adrinjalali pushed a commit to adrinjalali/scikit-learn that referenced this pull request May 11, 2020
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants