8000 DOC Adding dropdown for module 2.1 Gaussian Mixtures by punndcoder28 · Pull Request #26694 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC Adding dropdown for module 2.1 Gaussian Mixtures #26694

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

Merged

Conversation

punndcoder28
Copy link
Contributor

Reference Issues/PRs

Adding dropdowns for section 2.1 Gaussian Mixtures mentioned in #26617

What does this implement/fix? Explain your changes.

Added dropdowns for the following

  1. Pros and cons of class GaussianMixture
  2. Selecting the number of components
  3. Estimation algorithms
  4. Choice of initialization method
  5. Estimation algorigthm
  6. Pros and cons of variational inference
  7. The Dirichlet process

@github-actions
Copy link
github-actions bot commented Jun 24, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: cb5f224. Link to the linter CI: here

@lucyleeow
Copy link
Member

I think CI is failing due to these formatting issues:

The following documentation warnings may have been generated by PR #https://github.com/scikit-learn/scikit-learn/pull/26694:
/home/circleci/project/doc/modules/mixture.rst:96: WARNING: Field list ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:121: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:183: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:293: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:339: WARNING: Explicit markup ends without a blank line; unexpected unindent.

see: https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/47964/workflows/99fc1965-bfe3-49cd-9523-6efc4568d0ce/jobs/240165

@punndcoder28
Copy link
Contributor Author

Thanks @lucyleeow for pointing it out. I see that every line with the change related to |details-end| but weirdly enough do not see it for line 142 of the doc/modules/mixture.rst. Can you confirm if that is correct?

@punndcoder28 punndcoder28 force-pushed the docs/dropdown-gaussian-mixtures branch 2 times, most recently from faaaae0 to 743fa1f Compare June 26, 2023 13:15
@lucyleeow
Copy link
Member

Current formatting problems in CI

The following documentation warnings may have been generated by PR #https://github.com/scikit-learn/scikit-learn/pull/26694:
/home/circleci/project/doc/modules/mixture.rst:96: WARNING: Field list ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:122: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:185: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:296: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/home/circleci/project/doc/modules/mixture.rst:340: WARNING: Field list ends without a blank line; unexpected unindent.
The following documentation files may have been changed by PR #https://github.com/scikit-learn/scikit-learn/pull/26694:

You can check and confirm this yourself here: https://app.circleci.com/jobs/github/scikit-learn/scikit-learn/240319
Click on the red step them scroll to the bottom

@punndcoder28 punndcoder28 force-pushed the docs/dropdown-gaussian-mixtures branch from 743fa1f to 07acad3 Compare June 27, 2023 17:00
@punndcoder28
Copy link
Contributor Author

Thanks @lucyleeow with the latest commit, all checks have passed

Copy link
Member
@ArturoAmorQ ArturoAmorQ 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 @punndcoder28, here are a couple of comments.

@punndcoder28 punndcoder28 force-pushed the docs/dropdown-gaussian-mixtures branch 2 times, most recently from 10dc1e9 to 9e2bf12 Compare July 29, 2023 08:04
@punndcoder28 punndcoder28 requested a review from ArturoAmorQ July 29, 2023 09:58
@punndcoder28 punndcoder28 force-pushed the docs/dropdown-gaussian-mixtures branch from 9e2bf12 to 4288734 Compare July 29, 2023 12:07
Copy link
Member
@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Here is another batch of comments. Please try not to force-push your commits as that makes the reviewing process more difficult.

@punndcoder28
Copy link
Contributor Author

Here is another batch of comments. Please try not to force-push your commits as that makes the reviewing process more difficult.

Sure. I usually amend the commits and therefore force-push the changes. Let me know what the exactly process which is followed in the repository and I will ensure I follow that

@punndcoder28 punndcoder28 requested a review from ArturoAmorQ July 31, 2023 17:56

|details-end|

.. _dirichlet_process:
Copy link
Member

Choose a reason for hiding this comment

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

The dropdown seems to be breaking this cross-refrence:

image

I think we can revert this dropdown until the problem is fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for the late reply. Will remove the dropdown from Dirichlet Process. Where is this section exactly so that I can test after removing the drop-down feature

@punndcoder28 punndcoder28 force-pushed the docs/dropdown-gaussian-mixtures branch from f8132ca to 3f5d529 Compare August 29, 2023 16:11
@punndcoder28
Copy link
Contributor Author

Two required tasks

  1. ci/circleci: doc
  2. ci/circleci: doc-min-dependencies

were cancelled. Is this to be expected? Since they are marked as Required

@ArturoAmorQ
Copy link
Member

Two required tasks

1. ci/circleci: doc

2. ci/circleci: doc-min-dependencies

were cancelled. Is this to be expected?

It is indeed not expected. I am going to update your branch to see if that corrects the problem.

@punndcoder28
Copy link
Contributor Author

Seems like updating the PR with changes from main branch fixed failing tasks

Copy link
Member
@ArturoAmorQ ArturoAmorQ 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 @punndcoder28 for your work and patience :) merging!

@ArturoAmorQ ArturoAmorQ merged commit a6a6a47 into scikit-learn:main Sep 11, 2023
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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
0