update method layout docs and add a CI check for it - #219
Merged
Conversation
Since b5c5c96 (2024-07) a method has spanned two directories: algorithms/ holds the install spec that is baked into the docker image, and experiment/methods/ holds regressor.py, which is bind-mounted at run time. Before that, CI copied regressor.py from algorithms/ into experiment/methods/ via scripts/copy_algorithm_files.sh, so contributors only touched one place. That script is gone, but CONTRIBUTING.md and the PR template were never updated and still describe the old single-directory flow. Nothing catches the mistake. The build-and-test matrix is generated from `ls algorithms/`, so a method added only under experiment/methods/ never gets a matrix job and the PR shows a full set of green checks -- see #211, which has 29 checks where a correct submission has 30. Docs: - CONTRIBUTING.md gains a "Where your files go" section with the layout, the reason for the split, and local test commands. Fixes the regressor.py location, points at base_environment.yml, lists the recognized eval_kwargs keys and the five time-limit attribute names, and defines the [metadata] and [regressor] reference links, which were never defined and rendered as literal text. - The PR template is restructured around the two directories. - local_ci.sh ran the retired conda flow and copied regressor.py in the obsolete direction; it now wraps the docker build/test commands. - Small factual fixes in user_guide.md and README.md. Guardrail: - scripts/check_method_layout.py validates that the two directories agree, that regressor.py defines `est` and `model` (parsed via ast, so no dependencies are needed), and that install files are not misplaced. Legacy method dirs with no algorithms/ counterpart and the 10 empty metadata.yml files are grandfathered so the check passes on the current tree; new methods must satisfy both. - A validate-layout job runs it and gates build-and-test, so a misplaced submission fails in seconds instead of after ~27 docker builds. Three real problems this surfaced, fixed here: - algorithms/eql/metadata.yml was not valid YAML (unindented block scalar) - experiment/methods/xgboost/__init__.py was missing - gplearn's regressor.py used an invalid escape sequence, '\(' -> r'\(' Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lacava
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CONTRIBUTING.md and the PR template still describe the old single-directory layout, from before regressor.py moved into experiment/methods/ (b5c5c96, july 2024). back then CI copied the files over for you via scripts/copy_algorithm_files.sh, so contributors only touched algorithms/. that script is gone but the docs never got updated.
nothing catches the mistake either. the build-and-test matrix comes from
ls algorithms/, so a method added only under experiment/methods/ never gets a job and the PR still goes all green. that's #211, which has 29 checks where a correct submission has 30. #212 is missing the init.py that copy_algorithm_files.sh used to create automatically.docs:
check:
a few real problems it turned up, fixed here: