Skip to content

Latest commit

 

History

History
613 lines (477 loc) · 23.2 KB

CONTRIBUTING.md

File metadata and controls

613 lines (477 loc) · 23.2 KB

Contributing to dataprof

Thank you for considering contributing to dataprof! We welcome contributions from everyone. This document provides guidelines for contributing to the project.

Quick Start for Contributors

Prerequisites

  • Rust 1.96 or later (install). This is the MSRV; CI lints and tests on the toolchain in rust-toolchain.toml, which rustup installs and uses for cargo in this checkout on its own
  • Cargo (comes with Rust)
  • Python 3.11 or later. Local development runs on CPython 3.15, pinned in .python-version; uv reads that file and installs the interpreter itself. pyenv and asdf read it too, so the commands below start Python through uv
  • uv for Python dependency and test commands (install)
  • Git

Fork & Clone

  1. Fork the repository on GitHub

  2. Clone your fork:

    git clone https://github.com/YOUR-USERNAME/dataprof.git
    cd dataprof
  3. Add upstream remote:

    git remote add upstream https://github.com/AndreaBozzo/dataprof.git

Build & Test

Run the complete contributor setup smoke path from the repository root:

uv run --no-project python .github/scripts/contributor_smoke.py

The runner stops at the first failure and executes exactly these five steps:

# Install Python dev dependencies, without building dataprof itself
uv sync --no-install-project

# Build and install the local Python extension into the uv environment
uv run --no-sync maturin develop

# Run focused Python API tests
uv run --no-sync pytest python/tests/test_python_api.py -q

# Run focused Rust tests
cargo test -p dataprof-core
cargo test -p dataprof-python

uv sync on its own also installs dataprof itself, which builds the extension a first time through the PEP 517 backend, in release mode with full LTO; plain uv run then rebuilds it whenever maturin develop has replaced it. Hence --no-install-project once and --no-sync on every later uv run, as CI does (.github/workflows/ci.yml, "Build Python bindings"). A later uv sync uninstalls the extension maturin develop installed, so rebuild after it.

It does not lint or format. Those are separate, and CI runs them over the same paths, so run the ones covering what you changed before you open a PR. CI checks formatting with ruff format --check; locally you want the rewriting form below:

# Rust. rustup runs the toolchain rust-toolchain.toml pins, as CI does.
cargo fmt --all
cargo clippy --all --all-targets -- -D warnings
uv run --no-project python .github/scripts/check_decode_audit.py

# Python. The paths matter: CI checks the scripts directories too, so a
# narrower path can pass locally and still fail there.
uv run --no-sync ruff format python/ .github/scripts/ .claude/skills/dataprof/scripts/
uv run --no-sync ruff check python/ .github/scripts/ .claude/skills/dataprof/scripts/
uv run --no-sync ty check python/

You do not need to run every expensive workspace check for a small docs or Python-only PR. Prefer a focused test command that matches your change, and list exactly what you ran in the PR body.

Rust toolchains: MSRV vs pinned

Two Rust versions are in play and they do different jobs.

  • 1.96 is the MSRV, declared once as rust-version in the root Cargo.toml and inherited by every workspace package. It is a promise to downstream crates: the published libraries compile on it.
  • The development toolchain is pinned in rust-toolchain.toml, read by rustup locally, by .github/actions/setup-rust in CI and by the release build. Formatting, clippy and the test suites run there. Clippy gains lints with every release, so the pin moves deliberately: bump channel in that file and fix the new lints in the same change, so no unrelated PR goes red the day a Rust release lands.

The MSRV is verified by compilation, not by declaration:

rustup toolchain install 1.96
RUSTUP_TOOLCHAIN=1.96 uv run --no-project python .github/scripts/msrv_check.py

That script is the MSRV Compile Gate job in CI and a release gate in release.yml. It asserts every workspace package declares the same rust-version, asserts the running toolchain is that version — a gate run on a newer compiler proves nothing — and then runs cargo check --locked --lib over each published feature graph: the facade with no default features, with defaults, with async-streaming, with parquet-async, with all features (which covers database and the three connectors), and the dataprof-python extension crate, since a source install builds that from an sdist on the user's own toolchain.

Two scope choices worth knowing:

  • --lib. The promise covers the libraries a consumer depends on. Dev-dependencies, examples and benches are development tooling and build on the pinned toolchain instead.
  • --locked. The gate compiles the dependency set a release actually ships. It is not a minimal-version check; a dependency raising its own MSRV shows up when Cargo.lock changes, which is when a dependency-update PR runs the job.

Raising the MSRV is a deliberate, separate change: bump rust-version in Cargo.toml, the dtolnay/rust-toolchain@ pins and RUSTUP_TOOLCHAIN on the two MSRV jobs and the release metadata step, and the version stated in README.md, AGENTS.md and this file. RUSTUP_TOOLCHAIN is what keeps those jobs on the MSRV: rust-toolchain.toml outranks the default toolchain an install step sets. The script fails loudly if the toolchain pin and rust-version drift apart.

Python interpreters and release wheels

Starting with 0.12, .github/python-interpreters.json declares the release interpreter set: standard, GIL-enabled CPython 3.11–3.15. CI runs the Python suite on every declared version and wheel smoke tests on every declared version on Linux, macOS and Windows. PyPy, preview Python and free-threaded builds are outside this support policy; they are not published. Adopting free-threading or the stable ABI requires a separate decision and matching runtime coverage.

The release workflow installs the declared interpreters and resolves their absolute paths with uv python find on Windows and macOS. Linux builds select versioned names inside manylinux. Before uploading any build leg's wheels, it checks that each declared interpreter appears exactly once, that filename and archive ABI tags agree, and that Requires-Python matches the supported range. Both baseline and optimized builds pass this check before optimized artifacts are renamed. Linux wheels select interpreters inside the manylinux container; host Python installation does not control those builds (see the maturin action documentation). The aarch64 leg cross-compiles in a manylinux_2_28-cross image, where maturin takes each interpreter's configuration from its bundled sysconfig rather than from a target interpreter.

When changing support, update the JSON declaration and the version classifiers and requires-python in pyproject.toml, then refresh uv.lock. Metadata must agree with the declaration or both CI and the release fail. The bound excludes Python 3.16+ from source installs too; extending support is a tested release change, rather than an implicit promise made by an open-ended requirement. Local development follows the newest declared version through .python-version; CI jobs pinned to the floor set UV_PYTHON, so that file does not move them. Python package metadata cannot exclude the free-threaded ABI or PyPy by itself; the artifact check and this policy define that boundary.

uv run --no-sync python .github/scripts/wheel_interpreters.py
uv run --no-sync pytest python/tests/test_wheel_interpreters.py -q
# After building a complete set for one platform/CPU profile:
uv run --no-sync python .github/scripts/wheel_interpreters.py --wheels dist

Rehearsing the release build

The release builds its Python distributions in .github/workflows/python-dist.yml: every wheel leg and the sdist, then each baseline wheel installed into a bare venv on its own platform, one interpreter at a time (the cross-compiled aarch64 Linux wheels on an arm64 runner), and the sdist built into a package, each running .github/scripts/wheel_smoke.py. Until that workflow passes, a release has only its draft GitHub release: no asset is uploaded and nothing is published to crates.io or PyPI.

The Release Rehearsal workflow runs the same file and publishes nothing: no GitHub release, no upload, no crates.io or PyPI. Run it after changing the interpreter set, the shipped feature set ([tool.maturin] features), the build images or the maturin version:

gh workflow run release-rehearsal.yml --ref <branch>
gh run watch

A pull request that changes .github/python-interpreters.json, wheel_interpreters.py, rust-toolchain.toml, python-dist.yml or release-rehearsal.yml runs it on its own. The release builds with the newest maturin that [build-system] requires in pyproject.toml allows, in manylinux images selected by floating tags, so an upstream release changes the build without any change here. The rehearsal therefore also runs weekly on master; run it there before tagging as well.

Each install job's summary shows, per interpreter, whether the wheel was built, whether it installs and whether the smoke test passed; an interpreter maturin cannot resolve fails the build step before compilation.

Database regression tests

CI runs both the facade integration target and every test target in the database crate, including unit tests, column decoding, query column order, and typed PostgreSQL/MySQL decoding:

cargo test --test database_integration --features "all-db"
cargo test -p dataprof-db --all-features

Set POSTGRES_TEST_URL and MYSQL_TEST_URL to disposable test databases to run the service-backed cases, as the CI database job does. Without these variables, those cases return early; SQLite cases need no external service.

The separate Python database job builds SQLite support and runs the API, option parity, and column-order suites plus the SQLite engine-parity case. Check both the native exports and the compiled connector before pytest so a missing feature cannot turn these regressions into skipped tests:

uv run --no-sync maturin develop --features "python,python-async,sqlite"
uv run --no-sync python - <<'PY'
import dataprof
from dataprof._dataprof import (
    analyze_database_async,
    count_table_rows_async,
    get_table_schema_async,
    test_connection_async,
)

caps = dataprof.capabilities()
assert caps.database and "sqlite" in caps.database_connectors, caps
print(caps)
PY
uv run --no-sync pytest \
    python/tests/test_database_api.py \
    python/tests/test_database_option_parity.py \
    python/tests/test_column_order.py \
    python/tests/test_engine_parity.py::test_sqlite_parity -v -ra

--no-sync keeps the SQLite-enabled extension built by maturin develop in use. This feature set intentionally skips the three async file-transport cases in test_column_order.py; the ordinary Python job covers them. Every database case runs here. The ordinary wheel build omits database support.

Development Workflow

Create a Feature Branch

git checkout -b feature/your-feature-name

Use descriptive branch names:

  • feature/add-json-support - for new features
  • fix/handle-empty-files - for bug fixes
  • docs/update-api-reference - for documentation
  • perf/optimize-memory-usage - for performance improvements

Code Changes

  1. Make your changes - keep commits focused and logical
  2. Write tests - new features should have corresponding tests
  3. Test locally:
    uv run --no-sync pytest python/tests/test_python_api.py -q
    cargo test -p dataprof-core
    cargo test -p dataprof-python
  4. Format code:
    cargo fmt --all
  5. Lint checking:
    uv run --no-sync ruff check python/dataprof python/tests
    cargo clippy --all --all-targets -- -D warnings
    uv run --no-project python .github/scripts/check_decode_audit.py
  6. Build release (verify no warnings):
    cargo build --release

Commit Messages

Write clear, descriptive commit messages:

Short summary (50 chars max)

More detailed explanation if needed (wrap at 72 chars).
Explain what changed and why, not how.

Closes #123

Picking an Issue

If you are new to the project, start with issues labelled good first issue, difficulty: beginner, or mentor available. These are expected to have a small scope and enough context for a first PR.

Before opening a PR, leave a short comment on the issue saying which slice you want to take. Some larger issues are intentionally split into independent subtasks, so a focused partial PR is welcome.

Good first issues in this repository usually mean:

  • no deep knowledge of the Rust engine internals is required
  • the desired behavior is described in the issue body
  • maintainers are happy to answer API or setup questions
  • a small PR is better than a sweeping rewrite

Submit Pull Request

  1. Push to your fork:

    git push origin feature/your-feature-name
  2. Create PR on GitHub with:

    • Clear title describing the change
    • Description of what changed and why
    • Reference to related issues (e.g., "Closes #123")
    • Screenshots/examples for user-visible changes
    • A short list of the commands you ran
  3. CI will automatically run:

    • Tests
    • Code formatting checks
    • Lint checks
    • Build verification
  4. Address feedback - we may request changes before merging

Pull Request Guidelines

Before Submitting

✅ Checklist:

  • Code builds without warnings
  • Relevant tests pass
  • Code is formatted (cargo fmt --all)
  • No linting errors in the area you touched
  • Decode audit passes (uv run --no-project python .github/scripts/check_decode_audit.py); see Decode-path error handling
  • Added tests for new functionality
  • Updated documentation if needed
  • Commit messages are clear and descriptive

Report schema release checklist

When a change touches a serialized ProfileReport field or its Serde attributes:

  1. Decide whether the change is additive and compatible or requires a new REPORT_SCHEMA_VERSION.
  2. Run cargo run --example generate_profile_schema.
  3. Run cargo test --test profile_report_schema and the focused Python report tests.
  4. Review the generated diff for required fields, nullability, enum values, primitive types, and additive-property behavior.
  5. For an incompatible change, add a new versioned artifact and keep every schema still supported by readers.

PR Size

Small PRs are easier to review and merge. A good default is one behavior change or one documentation/example slice per PR. If an issue lists several scenarios, it is fine to implement one scenario and leave the rest for follow-ups.

Avoid mixing unrelated cleanup with feature work. If you notice something nearby, open a follow-up issue or separate PR.

PR Description Template

Opening a pull request fills the body in from .github/pull_request_template.md, so there is nothing to copy by hand. It asks for what changed, the related issue, and the commands you ran to verify it.

Run the checks that match what you touched rather than the whole workspace. The template collapses the usual ones per area into a <details> block, drawn from the lint and format commands in Build & Test above.

When the issue lists acceptance criteria, the template asks for one line per criterion: where the PR meets it, or why it leaves it out. Leaving one for a follow-up is fine when the PR says so.

Understanding Your Change

AI coding tools are welcome here. The PR is still yours, and review assumes you understand it:

  • Read the code the issue names before changing it, and derive the behaviour from that code. The issue describes intent; the code is what your change has to agree with.
  • Be able to explain every line of the diff and answer review questions yourself.
  • For a fix, cover the class of the defect: the reported case with the neighbouring values, input routes and report forms the same code serves. The template's "Class of defect" section lists what is covered and what is left out. A fix scoped to one case tends to come back as the next one.
  • Make every sentence of the PR description true of the diff. A claim such as "covers every input path" is something a reviewer will check.

A PR whose description does not match its diff, or that shows the code it changes was not read, gets one comment saying so instead of a line-by-line review. The full review follows once a revision addresses it.

Review Process

  1. Automated checks run first (format, lint, tests)
  2. Code review - maintainers will review your changes
  3. Feedback loop - we may ask for adjustments, but we try to explain the project constraint behind each request
  4. Merge - once approved and checks pass

Maintainer review style is direct but collaborative: expect requests to keep the public API small, preserve backward compatibility when possible, and add tests only where they protect real behavior.

Feature Requests and Bug Reports

🐛 Bug Reports

Before submitting, check if the issue already exists.

Include:

  • Rust version: rustc --version
  • OS and version: uname -a (Linux/Mac) or Windows version
  • Steps to reproduce the issue
  • Expected vs actual behavior
  • Sample data (anonymized if needed)
  • Error messages or stack traces

✨ Feature Requests

Describe:

  • What feature you need
  • Why you need it
  • Potential use cases
  • Any implementation ideas you have

Questions?

Code Guidelines

Architecture & Design

  • Modularity: Keep modules focused and single-purpose
  • Composition: Prefer composition over inheritance
  • Error Handling: Handle errors gracefully with meaningful messages
  • Performance: Consider memory usage for large files and streaming
  • Compatibility: Maintain backward compatibility when possible

Decode-path error handling

Run this check from the repository root before opening a PR:

uv run --no-project python .github/scripts/check_decode_audit.py

A decode or parse error must not silently become a plausible value such as 0, an empty string, or missing data. That would make the profile describe data that was never successfully read. Propagate the error with ?, adding context when useful, so the caller can see what failed.

The decode-audit checker guards .ok(), .unwrap_or_default(), and .unwrap_or(0...) in a deliberately limited set of Rust decode modules, listed in its AUDITED_PATHS. These patterns can hide failures, but some uses handle expected absence or proven invariants. For those uses, explain the case with this exact comment form on the same line or within the 10 preceding lines:

// decode-audit: <classification> — <reason>
Classification When it applies
impossible An established invariant proves the error cannot occur. State that invariant; prefer expect("reason") when an error would mean it was violated.
no-data Absence is an expected part of the operation, not a decode failure. Explain why the fallback represents that absence correctly.
unknown A failure is surfaced through an error or explicit diagnostic, and the unavailable result stays unknown. Explain where the failure is surfaced; logging it does not justify reporting a fabricated value.

For example, an empty delimiter-detection sample has no evidence for a candidate. A classified fallback over counts from already-read records is acceptable:

// decode-audit: no-data — no sampled records means zero delimiter evidence.
let evidence = candidate_counts.into_iter().max().unwrap_or(0);

By contrast, std::str::from_utf8(bytes).unwrap_or_default() would turn corrupt text into an empty value. This decode must propagate its error:

let text = std::str::from_utf8(bytes)?;

A classification comment explains why the code is correct; it does not make swallowing a decode error acceptable. The checker is a textual guard, so code review must still verify the reason. On failure, it prints each offending file:line and the matched source line. Fix the error handling or document a justified classification, then rerun the check. See #364 for the original rationale.

Testing Standards

For bounded parser and report fuzzing, run cargo test --manifest-path fuzz/Cargo.toml --locked --test smoke. The fuzzing guide covers stable Windows-compatible corpus replay, pinned Linux sanitizer campaigns, retained artifacts, and minimizing findings into ordinary regression tests.

  • Unit tests: Test individual functions and modules
  • Integration tests: Test end-to-end workflows
  • Edge cases: Test with:
    • Empty files
    • Very large files
    • Malformed data
    • Different encodings
    • Special characters

Example:

#[test]
fn test_handles_empty_file() {
    let result = process_csv("");
    assert!(result.is_err());
}

Documentation

  • Document public APIs with doc comments
  • Include examples in doc comments
  • Update README.md for user-facing changes
  • Update the documentation directory for architectural changes

Development Tips

Useful Commands

# Install/update the Python dev environment, without building dataprof itself
uv sync --no-install-project

# Rebuild the local Python extension after Rust/PyO3 changes
uv run --no-sync maturin develop

# Run the main Python API regression file
uv run --no-sync pytest python/tests/test_python_api.py -q

# Run a focused Python test
uv run --no-sync pytest python/tests/test_llm_context.py::TestToLlmContext -v -ra

# Generate documentation
cargo doc --open

# Run targeted Rust package tests
cargo test -p dataprof-core
cargo test -p dataprof-python

# Run lint checks
uv run --no-sync ruff check python/dataprof python/tests
cargo clippy --all --all-targets -- -D warnings
uv run --no-project python .github/scripts/check_decode_audit.py

# Run benchmarks
cargo bench

# Run library checks
cargo check --workspace --all-targets

The benchmark suite inventory, reproducible Python comparison command, and shared CI-to-website artifact contract live in benches/README.md. Keep new benchmark scenarios within that structure so they remain publishable alongside the existing Criterion reports.

Project Structure

  • crates/dataprof/src/ - Public dataprof facade source
  • crates/ - Workspace crates that own core, parser, engine, database, metrics, and runtime code
  • crates/dataprof-db/ - Database connectors
  • crates/dataprof-metrics/ - Core analysis and statistics logic
  • crates/dataprof-python/ - PyO3 extension crate
  • python/ - Python package sources and tests
  • tests/ - Integration tests
  • docs/ - Documentation

License

By contributing, you agree that your contributions will be licensed under the same dual license as the project: MIT OR Apache-2.0.