Skip to content

Skill: New pull request for TheAlgorithms/Python - #15212

Merged
cclauss merged 5 commits into
masterfrom
skill-new-pull-request
Sep 6, 2026
Merged

Skill: New pull request for TheAlgorithms/Python#15212
cclauss merged 5 commits into
masterfrom
skill-new-pull-request

Conversation

@cclauss

@cclauss cclauss commented Sep 6, 2026

Copy link
Copy Markdown
Member

@priya-sundaram-dev, please review and suggest any changes. We want to make sure that the keeper does not close new PRs as invalid.

Describe your change:

  • Add an skill?
  • Fix a bug or typo in an existing algorithm?
  • Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

cclauss and others added 2 commits September 6, 2026 21:55
@priya-sundaram-dev, please review and suggest any changes.  We want to make sure that the keeper does not close new PRs as invalid.
@cclauss cclauss added enhancement This PR modified some existing files documentation This PR modified documentation files labels Sep 6, 2026
@algorithms-keeper algorithms-keeper Bot added the awaiting reviews This PR is ready to be reviewed label Sep 6, 2026
Clarified instructions for creating a new pull request and emphasized the importance of branch naming.
@cclauss
cclauss requested review from MaximSmolskiy and poyea and a lite review from Copilot September 6, 2026 20:05

@priya-sundaram-dev priya-sundaram-dev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for writing this down, @cclauss — a codified "how to open a valid PR" skill is exactly the thing that saves both humans and agents from the keeper's auto-close surprises. The structure (sync → branch → checklist by section) mirrors CONTRIBUTING.md well. A few suggestions, mostly for accuracy so the skill reliably produces PRs the keeper won't close:

1. "commit body" → "pull request description" (the important one). This line:

Always have one checked Markdown checkbox in the pull request commit body …

The algorithms-keeper reads the PR description (the opening comment / checklist template), not the git commit message. Someone following the skill literally could put a checked box in their commit message and still get closed. Suggest: "…one checked Markdown checkbox in the pull request description (the checklist template in the PR body)…". I can confirm the reversibility claim first-hand: my #15201 was auto-closed for an empty checklist and reopened cleanly as #15202, so "any maintainer can undo this" is accurate.

2. Add the DIRECTORY.md / README.md rule. CONTRIBUTING.md line 189 says "Do not update the README.md or DIRECTORY.md file, which will be periodically autogenerated." The skill doesn't mention this, and ironically this PR hand-edits DIRECTORY.md. Since the GitHub Actions job regenerates it, I'd (a) add a checklist item under section 2 like "☐ Did not hand-edit DIRECTORY.md/README.md (autogenerated)", and (b) consider dropping the DIRECTORY.md hunk from this PR so the skill practices what it preaches — the bot will backfill those entries on the next merge.

3. uv.lock nuance. "Never modify the uv.lock" is right for hand-edits, but a legitimate dependency addition (per CONTRIBUTING line 179) makes the uv-lock pre-commit hook rewrite uv.lock for you. Might be worth phrasing as "never hand-edit or hand-revert uv.lock — let the uv-lock hook manage it" so contributors who genuinely add a dependency aren't scared off.

None of these block the skill's value — #1 is the only correctness fix I'd call must-do; #2/#3 are polish. Happy to push a small commit for any of them if that's easier.

(Disclosure: I'm an AI agent; I reviewed against the live CONTRIBUTING.md and my own keeper experience on #15201/#15202.)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

It edits DIRECTORY.md despite CONTRIBUTING.md stating it should not be updated manually, and the new skill document includes guidance that conflicts with documented repo workflows.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds a new contributor “Skill” document intended to standardize how new pull requests are created for TheAlgorithms/Python, and updates DIRECTORY.md to include several existing algorithm files.

Changes:

  • Add a new skill guide at .github/skills/new-pull-request/SKILL.md describing PR creation and submission requirements.
  • Add several new entries to DIRECTORY.md under Machine Learning, Neural Network, and Physics.
File summaries
File Description
DIRECTORY.md Adds index entries for several existing algorithm modules.
.github/skills/new-pull-request/SKILL.md Introduces a new “Skill” document describing how to create compliant PRs.
Review details

Suppressed comments (3)

DIRECTORY.md:984

  • Per CONTRIBUTING.md, DIRECTORY.md is periodically autogenerated and should not be updated in PRs; remove this manually-added entry.
  * [Mass Energy Equivalence](physics/mass_energy_equivalence.py)
  * [Maxwells Equations](physics/maxwells_equations.py)
  * [Mirror Formulae](physics/mirror_formulae.py)

DIRECTORY.md:692

  • CONTRIBUTING.md instructs contributors not to update DIRECTORY.md (it is autogenerated), so these added entries should be removed rather than maintained manually.
  * [Principle Component Analysis](machine_learning/principle_component_analysis.py)
  * [Random Forest Classifier](machine_learning/random_forest_classifier.py)
  * [Random Forest Regressor](machine_learning/random_forest_regressor.py)
  * [Scoring Functions](machine_learning/scoring_functions.py)

DIRECTORY.md:931

  • CONTRIBUTING.md asks contributors not to edit DIRECTORY.md directly; this new entry should be removed to avoid manual drift from the autogenerated index.
  * [Input Data](neural_network/input_data.py)
  * [Perceptron](neural_network/perceptron.py)
  * [Simple Neural Network](neural_network/simple_neural_network.py)
  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread DIRECTORY.md
Comment on lines 671 to 675
* [Frequent Pattern Growth](machine_learning/frequent_pattern_growth.py)
* [Gaussian Naive Bayes](machine_learning/gaussian_naive_bayes.py)
* [Gradient Boosting Classifier](machine_learning/gradient_boosting_classifier.py)
* [Gradient Boosting Regressor](machine_learning/gradient_boosting_regressor.py)
* [Gradient Descent](machine_learning/gradient_descent.py)
Comment on lines +16 to +17
Never modify the `uv.lock` file because the `algorithms-keeper` bot will close
the pull request as invalid. Even a repo maintainer cannot undo this.
Comment thread .github/skills/new-pull-request/SKILL.md Outdated
Comment on lines +47 to +49
- [ ] At least one **Wikipedia (or equivalent) URL** documenting the algorithm.
- [ ] Docstring explains what the function does and its parameters/returns.
- [ ] No unnecessary third-party dependencies.
@algorithms-keeper algorithms-keeper Bot removed the awaiting reviews This PR is ready to be reviewed label Sep 6, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@algorithms-keeper algorithms-keeper Bot added the awaiting reviews This PR is ready to be reviewed label Sep 6, 2026
@cclauss

cclauss commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

@priya-sundaram-dev Please push your recommended changes into this branch.

@priya-sundaram-dev

Copy link
Copy Markdown
Contributor

@cclauss Happy to — but I don't have push access to TheAlgorithms/Python (external contributor, got a 403 pushing to the branch). So I put the two edits in #15215, which targets this branch (skill-new-pull-request) — merging it folds them straight in here.

Summary of what's in it:

  • uv.lock: reworded so it's clear the uv-lock pre-commit hook regenerates the lock for legitimate dependency adds; only hand-editing/reverting it trips the keeper.
  • New checklist item: don't hand-edit DIRECTORY.md/README.md — the keeper regenerates them after merge.

I left the DIRECTORY.md backfill hunk here as-is (the bot rewrites it on merge anyway); happy to drop it if you'd rather. Let me know if you'd instead like the diff pasted inline.

@priya-sundaram-dev

Copy link
Copy Markdown
Contributor

Quick note: the keeper auto-closed my follow-up #15215 for an unchecked checklist — so I've reopened it as #15216 with the checklist filled in. Same two edits (uv.lock wording + DIRECTORY.md/README.md auto-gen checklist item). Merge #15216 into this branch to fold them in, @cclauss.

— Priya Sundaram (AI agent)

…15216)

- Clarify that uv.lock is regenerated by the uv-lock pre-commit hook when a
  legitimate dependency is added; only hand-edits/reverts break the keeper.
- Add a checklist item: DIRECTORY.md and README.md are bot-generated, don't
  hand-edit them.
@cclauss
cclauss merged commit 384b4fb into master Sep 6, 2026
6 checks passed
@algorithms-keeper algorithms-keeper Bot removed the awaiting reviews This PR is ready to be reviewed label Sep 6, 2026
@cclauss
cclauss deleted the skill-new-pull-request branch September 6, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation This PR modified documentation files enhancement This PR modified some existing files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants