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: Add info about incremental algorithms and BS (#2103) #2112

Merged

Conversation

olegkkruglov
Copy link
Contributor

@olegkkruglov olegkkruglov commented Oct 15, 2024

  • Add 'Non-Scikit algorithms' part to the docs
  • Add info about IncrementalPCA
  • Add sphinx.napoleon extension to generate docs from docstrings.
  • Update docstrings for non-scikit algorithms

Description

Add a comprehensive description of proposed changes

List associated issue number(s) if exist(s): #6 (for example)

Documentation PR (if needed): #1340 (for example)

Benchmarks PR (if needed): IntelPython/scikit-learn_bench#155 (for example)


Checklist to comply with before moving PR from draft:

PR completeness and readability

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with update and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have added a respective label(s) to PR if I have a permission for that.
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.
  • I have extended testing suite if new functionality was introduced in this PR.

Performance

  • I have measured performance for affected algorithms using scikit-learn_bench and provided at least summary table with measured data, if performance change is expected.
  • I have provided justification why performance has changed or why changes are not expected.
  • I have provided justification why quality metrics have changed or why changes are not expected.
  • I have extended benchmarking suite and provided corresponding scikit-learn_bench PR if new measurable functionality was introduced in this PR.

- Add 'Non-Scikit algorithms' part to the docs
- Add info about IncrementalPCA
- Add sphinx.napoleon extension to generate docs from docstrings.
- Update docstrings for non-scikit algorithms
@samir-nasibli
Copy link
Contributor

fails in this PR is not related with the changes.
Already resolved for the main branch with test deselections.
Could be ignored in the scope of this PR validation

samir-nasibli
samir-nasibli previously approved these changes Oct 15, 2024
@samir-nasibli
Copy link
Contributor

@maria-Petrova here only docstrings are changed do we need to run internal CI?

@maria-Petrova
Copy link
Contributor

@samir-nasibli, doc build in public CI is sufficient. Why condaEnv steps are failing?

@samir-nasibli
Copy link
Contributor

samir-nasibli commented Oct 15, 2024

Potentially this changes dependend on the PR #2038
I am not sure what is better partially bring those changes here, for BS attr renaming I mean or just update docstrings for the 2025.0 BS impl. From my prospective the second one more safe.
@maria-Petrova @icfaust I would like to know your opinions as well

@samir-nasibli
Copy link
Contributor

@samir-nasibli, doc build in public CI is sufficient. Why condaEnv steps are failing?

I have already comment out, but now it comes to me that my prev comment #2112 (comment) is not correct. I see fails, that already should be addressed on 2025.0 rls branch with the latest commits there.
@Vika-F @ethanglaser any thoughts?

@ethanglaser
Copy link
Contributor

It should be merged, but would prefer to merge via backport PR (#2114 )

@samir-nasibli
Copy link
Contributor

@olegkkruglov what is the status?

@samir-nasibli samir-nasibli merged commit 65afadb into uxlfoundation:rls/2025.0.0-rls Oct 22, 2024
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants