-
Notifications
You must be signed in to change notification settings - Fork 2
Add CONTRIBUTING.md #89
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
Open
arpitjain099
wants to merge
4
commits into
dbetchkal:main
Choose a base branch
from
arpitjain099:docs/contributing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
47db2af
Add CONTRIBUTING.md with setup and PR guidelines
arpitjain099 2316061
Replace boilerplate CONTRIBUTING.md and add AGENTS.md style guide.
elliott-ruebush fb4816b
Additional tweaks to AGENTS.md and CONTRIBUTING.md
elliott-ruebush 7e9c002
add bullet point on timezones
elliott-ruebush File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # NPS-ActiveSpace — Code conventions for humans and LLM agents | ||
|
|
||
| NPS-ActiveSpace is a scientific Python package for modeling geographic extent of audibility from a given point using sound pressure level and causal data. It also includes related library functionality and visualization capabilities. | ||
|
|
||
| See [README.md](README.md) for detailed background and installation instructions. | ||
|
|
||
| See [CONTRIBUTING.md](CONTRIBUTING.md) for PR workflow. | ||
|
|
||
| ## Repo layout | ||
|
|
||
| - **Source**: `nps_active_space/` at the repo root. The codebase includes general library utils as well as workflow scripts (under `scripts/`). | ||
| - **Tests**: `tests/` — mirrors source paths; test file names mirror source modules | ||
| - **Config**: ConfigParser `.config` files in `nps_active_space/config/`; copy `template.config` to `<environment>.config` and pass `-e <environment>` to scripts. | ||
| - **Scripts**: run from the repo root, e.g. `python nps_active_space/scripts/run_ground_truthing.py -e DENA ...`. See [nps_active_space/scripts/README.md](nps_active_space/scripts/README.md) for more details on each script + example invocations. | ||
|
|
||
| ## General Code Conventions | ||
| - Scientific logic lives in library modules (`active_space/`, `ground_truthing/`, `utils/`, `validation/`). Scripts handle argparse, config, then pass data and paths into library functions. | ||
| - Prefer concise, single-responsibility functions, classes, and modules. If one piece of logic gets too large, break it up into multiple clearly-named smaller ones. | ||
| - Don't leave comments that just restate the code. They should be used for scientific concepts, non-obvious geospatial operations, or to explain decisions. | ||
| - Units should be clearly specified in variable names (e.g., `altitude_m`, `EVENT_GAP_SECONDS`, `elapsed_seconds`). Unit conversions should live in clearly named functions (e.g. `feet_to_meters`, `seconds_to_hours`) | ||
|
|
||
|
|
||
| ## Python Code Conventions | ||
| - Target Python 3.12. | ||
| - Use modern type hints for all function signatures: `list[X]`, `dict[str, X]`, `X | None`. Not `typing.List`, `typing.Optional`. | ||
| - Prefer `match`/`case` for handling enum values or exhaustive case matching. | ||
| - Name DataFrames for what they hold (e.g. `sorted_mxak_points`, `flight_tracks`), not just `df`. | ||
| - Use `pathlib` and ensure cross-platform path support for any code dealing with file system paths. | ||
| - Timestamps are currently stored as tz-naive. Data parsers should return UTC-naive. See our [UTC standardization issue](https://github.com/dbetchkal/NPS-ActiveSpace/issues/96) for desired future state. | ||
|
|
||
| ## Testing | ||
|
|
||
| - `pytest` from the repo root with an activated venv. | ||
| - Group related tests into classes (`class TestX:`). | ||
| - DataFrames: use pandas or geopandas `testing.assert_frame_equal` against an expected result for test assertions on dataframe results. For narrow tests, assert a single field or created an expected dataframe with a subset of columns. | ||
| - New logic should ship with tests. If something is difficult to test in a unit test (physics model subprocess, GUI, external binaries), mention it in the PR description explaining why. | ||
| - Document any manual testing/verification that you do in PR descriptions. | ||
|
|
||
| ## Git | ||
| - Keep commits logically scoped. | ||
| - After a squash-merge, rebase dependent PRs with the `--onto` flag to minimize merge conflicts: `git rebase --onto <new-base> <old-base> <branch>`. | ||
|
|
||
| ## Additional agent instructions | ||
|
|
||
| - Activate `.venv` before running Python commands. | ||
| - Run `pytest` after code changes. Don't mark work complete without passing tests. | ||
| - Keep changes small and scoped. Don't refactor unrelated code or rename/move files without maintainer sign-off. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # Contributing | ||
|
|
||
| If you're not sure where to start, take a scan through our [GitHub Issues](https://github.com/dbetchkal/NPS-ActiveSpace/issues) and see if anything catches your eye! | ||
|
|
||
| If you're interested in making a larger-scale change, please consider creating a new issue or commenting on existing one to discuss with the repo maintainers. We'd love to meet you and hear about what you're interested in working on! | ||
|
|
||
| ## Setup | ||
|
|
||
| Install as described in the [README](README.md#installation). Use `pip install -e ".[dev]"` to include test tools. | ||
|
|
||
| ## Workflow | ||
|
|
||
| 1. Fork and create a branch — `feature/description` or `fix/description`. | ||
| 2. New logic should ship with tests. See [Style Guide](AGENTS.md) for more details. | ||
| 3. Ensure existing and new tests pass before marking a PR ready for review. GitHub actions will run against PRs pointing at main. | ||
| 4. Open a pull request against `main` with a clear title and description of what changed and why. Reference any related issues. | ||
|
|
||
| Prefer small, focused PRs with a concise, clear description. | ||
|
|
||
| ## Code style | ||
|
|
||
| See [`AGENTS.md`](AGENTS.md) for Python conventions, naming, type hints, testing patterns, and git workflow. | ||
|
|
||
| ## License | ||
|
|
||
| This project is [public domain (CC0)](LICENSE.md). By submitting a pull request you waive any copyright interest in your contribution. | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pip install -e ".[dev]" is pending the merge of #87
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, good call. I'll update this to reference the current setup steps until #87 lands, then we can swap it in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, I'll plan to merge #87 first, then merge #89