Skip to content

DOC take Examples out of a dropdown #27034

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
merged 7 commits into from
Sep 11, 2023
Merged

DOC take Examples out of a dropdown #27034

merged 7 commits into from
Sep 11, 2023

Conversation

hiramatsuyuusuke
Copy link
Contributor

@hiramatsuyuusuke hiramatsuyuusuke commented Aug 8, 2023

Reference Issues/PRs

(#26617, #26641)

What does this implement/fix? Explain your changes.

I fix scikit-learn\doc\modules\svm.rst.
A drop down of Custom Kernels in 1.4. Support Vector Machines(#26617) folds a Examples.
So I take a Examples out of a dropdown.
Then I added a drop down of Using the Gram matrix .
See also . #26641.

Any other comments?

There is a folded Examples in 1.4. Support Vector Machines.
So, I worked on 1.4. Support Vector Machines to fix a folded Examples.
I think #26641 was finish. So, I think this is a correct handover.

@github-actions
Copy link

github-actions bot commented Aug 8, 2023

✔️ Linting Passed

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

Generated for commit: 2f915e4. Link to the linter CI: here

@hiramatsuyuusuke
Copy link
Contributor Author

Sorry, I updated 4 times, because I did not understand the update work flow.
I will never do the meaningless multiple update.

@hiramatsuyuusuke
Copy link
Contributor Author

A message "This branch is out-of-date with the base branch" appears.
But doing "Update branch" results in redoing some checks.
Should I do "Update branch"?

@ArturoAmorQ
Copy link
Member

Thanks for the PR @hiramatsuyuusuke. Maybe you can convert the section 1.4.6.3. Using Python functions as kernels into a separate dropdown and then move the example out of it and after the Gram matrix dropdown, yet inside the 1.4.6.2. Custom Kernels. I hope that is clear, but if I didn't express correctly please let me know :)

Should I do "Update branch"?

In general you don't have do it, except is the CI is failing on our side or if the file concerned by the PR has conflicts. In any other case triggering the tests just consumes computational resources.

@hiramatsuyuusuke
Copy link
Contributor Author

hiramatsuyuusuke commented Aug 18, 2023

Thanks for the review and advice, @ArturoAmorQ.
I'm working on it.

And if there are no mistakes, I follow these steps:
When I complete it, I will push it to my fork branch.
Next, I will run "Update branch" botton on this page.

@hiramatsuyuusuke
Copy link
Contributor Author

hiramatsuyuusuke commented Aug 18, 2023

To reviewer @ArturoAmorQ.
I made some changes. Please confirm the these changes.

I made a separate dropdown Using Python functions as kernels . And I moved the example after the Gram matrix dropdown. Then two dropdowns and one example are inside the 1.4.6.2. Custom Kernels.

Additionally, I trigger the tests, because the file was changed. Were these tests necessary?

@ArturoAmorQ ArturoAmorQ changed the title DOC take Examples out of a dropdown (#26617, #26641) DOC take Examples out of a dropdown Sep 11, 2023
@ArturoAmorQ
Copy link
Member

Additionally, I trigger the tests, because the file was changed. Were these tests necessary?

Sorry for taking so long to answer. Triggering the tests is not necessary if you changed the file, it is only necessary if the file was changed by another PR and it is generating a conflict.

You can also re-trigger the tests if they are failing and the error message does not seem related to your changes.

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.

In any case this PR LGTM :) thanks for your work @hiramatsuyuusuke, merging this one!

@ArturoAmorQ ArturoAmorQ merged commit 7ba963f into scikit-learn:main Sep 11, 2023
@hiramatsuyuusuke hiramatsuyuusuke deleted the sklearn_DD_test branch September 11, 2023 08:58
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
jeremiedbb pushed a commit that referenced this pull request Sep 20, 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.

2 participants