Skip to content

DOC Adds example to OAS#16681

Merged
thomasjpfan merged 6 commits intoscikit-learn:masterfrom
marenwestermann:oas
Mar 13, 2020
Merged

DOC Adds example to OAS#16681
thomasjpfan merged 6 commits intoscikit-learn:masterfrom
marenwestermann:oas

Conversation

@marenwestermann
Copy link
Copy Markdown
Member

Reference Issues/PRs

Towards #3846

What does this implement/fix? Explain your changes.

Fixes missing examples in covariance.OAS documentation in #3846.
It is almost the same as covariance.ShrunkCovariance.

ping @adrinjalali

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @marenwestermann

>>> oas = OAS().fit(X)
>>> oas.covariance_
array([[0.7533..., 0.2763...],
[0.2763..., 0.3964...]])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think adding a few spaces here to align the brackets with the previous line would make it look nicer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree. Fixed it.

[0.2763..., 0.3964...]])
>>> oas.precision_
array([[ 1.7833..., -1.2431... ],
[-1.2431..., 3.3889...]])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

here as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, fixed it.

Comment on lines +546 to +547
... cov=real_cov,
... size=500)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I corrected the alignment here, too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But the correct alignment here is for cov to be aligned with mean, not the bracket, since it's not a part of the list represented by the brackets.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, true! I addressed it.

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @marenwestermann

@adrinjalali adrinjalali changed the title DOC add example to OAS [MRG+1] DOC add example to OAS Mar 13, 2020
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.

@marenwestermann I see this is your first time contributing, welcome!

LGTM

@thomasjpfan thomasjpfan changed the title [MRG+1] DOC add example to OAS DOC Adds example to OAS Mar 13, 2020
@thomasjpfan thomasjpfan merged commit bd36ada into scikit-learn:master Mar 13, 2020
@marenwestermann marenwestermann deleted the oas branch March 13, 2020 14:43
ashutosh1919 pushed a commit to ashutosh1919/scikit-learn that referenced this pull request Mar 13, 2020
@Malesche Malesche mentioned this pull request Mar 29, 2020
23 tasks
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 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