|
| 1 | +# Repository instructions for datafusion-python |
| 2 | + |
| 3 | +## Style and Linting |
| 4 | +- Python formatting and linting is handled by **ruff**. |
| 5 | +- Follow [PEP 8](https://www.python.org/dev/peps/pep-0008/) and these key |
| 6 | + ruff rules: |
| 7 | + - Use double quotes for string literals |
| 8 | + - Use explicit relative imports such as `from .module import Class` |
| 9 | + - Limit lines to 88 characters |
| 10 | + - Provide type hints for parameters and return values |
| 11 | + - Avoid unused imports |
| 12 | + - Prefer f-strings over other string formatting |
| 13 | + - Avoid `else` blocks after `return` |
| 14 | + - Use `isinstance()` instead of direct type comparison |
| 15 | + - Keep docstrings concise in Google format |
| 16 | + - Assign exception messages to variables before raising |
| 17 | +- All modules, classes, and functions should include docstrings. |
| 18 | +- Tests must use descriptive function names, pytest style assertions, and be |
| 19 | + grouped in classes when appropriate. |
| 20 | +- Rust code must pass `cargo fmt`, `cargo clippy`, and `cargo tomlfmt`. |
| 21 | +- Install the pre-commit hooks with `pre-commit install` and run them with |
| 22 | + `pre-commit run --files <file1> <file2>` or `pre-commit run --all-files`. |
| 23 | + |
| 24 | +Install the required Rust components before running pre-commit: |
| 25 | + |
| 26 | +```bash |
| 27 | +rustup component add rustfmt clippy |
| 28 | +``` |
| 29 | + |
| 30 | +If installation is blocked by a proxy, see the [offline installation guide](https://rust-lang.github.io/rustup/installation/other.html). |
| 31 | + |
| 32 | +- The same checks can be run manually using the scripts in `ci/scripts`: |
| 33 | + - `./ci/scripts/python_lint.sh` |
| 34 | + - `./ci/scripts/rust_fmt.sh` |
| 35 | + - `./ci/scripts/rust_clippy.sh` |
| 36 | + - `./ci/scripts/rust_toml_fmt.sh` |
| 37 | + |
| 38 | +## Rust Code Style |
| 39 | + |
| 40 | +When generating Rust code for DataFusion, follow these guidelines: |
| 41 | + |
| 42 | +1. Do not add unnecessary parentheses around `if` conditions: |
| 43 | + - Correct: `if some_condition` |
| 44 | + - Incorrect: `if (some_condition)` |
| 45 | + |
| 46 | +2. Do not use `expect()` or `panic!()` except in tests - use proper error handling with `Result` types |
| 47 | + |
| 48 | +3. Follow the standard Rust style conventions from rustfmt |
| 49 | + |
| 50 | +## Code Organization |
| 51 | +- Keep functions focused and under about 50 lines. |
| 52 | +- Break complex tasks into well-named helper functions and reuse existing |
| 53 | + helpers when possible. |
| 54 | +- Prefer adding parameters to existing functions rather than creating new |
| 55 | + versions with similar behavior. |
| 56 | +- Avoid unnecessary abstractions and follow established patterns in the |
| 57 | + codebase. |
| 58 | + |
| 59 | +## Comments |
| 60 | +- Add meaningful comments for complex logic and avoid obvious comments. |
| 61 | +- Start inline comments with a capital letter. |
| 62 | + |
| 63 | +## Running Tests |
| 64 | +1. Ensure submodules are initialized: |
| 65 | + ```bash |
| 66 | + git submodule update --init |
| 67 | + ``` |
| 68 | +2. Create a development environment and install dependencies: |
| 69 | + ```bash |
| 70 | + uv sync --dev --no-install-package datafusion |
| 71 | + ``` |
| 72 | +3. Build the Python extension: |
| 73 | + ```bash |
| 74 | + uv run --no-project maturin develop --uv |
| 75 | + ``` |
| 76 | +4. Execute the test suite: |
| 77 | + ```bash |
| 78 | + uv run --no-project pytest -v . |
| 79 | + ``` |
| 80 | + |
| 81 | +## Building Documentation |
| 82 | +- Documentation dependencies can be installed with the `docs` group: |
| 83 | + ```bash |
| 84 | + uv sync --dev --group docs --no-install-package datafusion |
| 85 | + ``` |
| 86 | +- Build the docs with: |
| 87 | + ```bash |
| 88 | + uv run --no-project maturin develop --uv |
| 89 | + uv run --no-project docs/build.sh |
| 90 | + ``` |
| 91 | +- The generated HTML appears under `docs/build/html`. |
| 92 | + |
| 93 | +## PR Template |
| 94 | + |
| 95 | +When creating a pull request, please use the following template: |
| 96 | + |
| 97 | +```markdown |
| 98 | +## Which issue does this PR close? |
| 99 | + |
| 100 | +<!-- |
| 101 | +We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. |
| 102 | +--> |
| 103 | + |
| 104 | +- Closes #. |
| 105 | + |
| 106 | +## Rationale for this change |
| 107 | + |
| 108 | +<!-- |
| 109 | + Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. |
| 110 | + Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. |
| 111 | +--> |
| 112 | + |
| 113 | +## What changes are included in this PR? |
| 114 | + |
| 115 | +<!-- |
| 116 | +There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. |
| 117 | +--> |
| 118 | + |
| 119 | +## Are these changes tested? |
| 120 | + |
| 121 | +<!-- |
| 122 | +We typically require tests for all PRs in order to: |
| 123 | +1. Prevent the code from being accidentally broken by subsequent changes |
| 124 | +2. Serve as another way to document the expected behavior of the code |
| 125 | + |
| 126 | +If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? |
| 127 | +--> |
| 128 | + |
| 129 | +## Are there any user-facing changes? |
| 130 | + |
| 131 | +<!-- |
| 132 | +If there are user-facing changes then we may require documentation to be updated before approving the PR. |
| 133 | +--> |
| 134 | + |
| 135 | +<!-- |
| 136 | +If there are any breaking changes to public APIs, please add the `api change` label. |
| 137 | +--> |
| 138 | +``` |
0 commit comments