Skip to content

Commit 310df18

Browse files
timsaucerclaude
andcommitted
docs(skills): make-pythonic also targets functions.spark
Previous guidance said to skip the spark namespace entirely. That was wrong: the spark namespace should also feel pythonic — it just carries the extra constraint that every signature must remain compatible with pyspark.sql.functions (parameter names, positional order, accepted input types). Pythonic widenings like `Expr → Expr | int` are on-brand there because pyspark itself accepts the int form. Rewrite the scope section to spell out the compatibility rules (keep parameter names/order; widen input types, never narrow; extra kwargs default to None) and extend "How to Identify Candidates" to include `functions/spark.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 43ecf1a commit 310df18

1 file changed

Lines changed: 33 additions & 19 deletions

File tree

.ai/skills/make-pythonic/SKILL.md

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,42 @@ You are improving the datafusion-python API to feel more natural to Python users
3131

3232
## Scope: `functions` vs `functions.spark`
3333

34-
This skill targets the **default `datafusion.functions` namespace** (file:
35-
`python/datafusion/functions/__init__.py`). Do **not** apply pythonic
36-
coercion to `python/datafusion/functions/spark.py` — that namespace is a
37-
deliberate mirror of `pyspark.sql.functions`, so its parameter names,
38-
order, and types must match pyspark exactly. Adding `Expr | int` style
39-
unions there would diverge from the pyspark contract callers rely on.
40-
41-
Two exceptions where pythonic-style additions in `functions.spark` are
42-
still on-brand:
43-
- **Pyspark itself accepts a native type.** Pyspark's `format_string`
44-
takes `format: str | Column`; the spark wrapper already auto-promotes a
45-
plain `str` to a literal — keep parity.
46-
- **Strictly additive optional kwargs** that pyspark also has (e.g.
47-
`like(escapeChar=...)`). These belong in pyspark-alignment follow-up
48-
PRs, not in a make-pythonic pass.
49-
50-
If the user explicitly scopes to "spark", validate parity with pyspark
51-
rather than applying generic coercion.
34+
Both `python/datafusion/functions/__init__.py` and
35+
`python/datafusion/functions/spark.py` are in scope. We want both to feel
36+
pythonic — accept native Python types where the argument is contextually
37+
a literal — but `functions.spark` carries an additional constraint:
38+
**every signature must remain compatible with `pyspark.sql.functions`**.
39+
40+
Compatibility rules for the spark namespace:
41+
42+
- **Parameter names must match pyspark exactly.** Pyspark callers pass by
43+
keyword (`spark.shiftleft(col=..., numBits=...)`), so renames break
44+
them. Do NOT rename a parameter just because it would be more pythonic
45+
in the main namespace.
46+
- **Positional order must match pyspark exactly.** Reordering breaks
47+
positional pyspark calls.
48+
- **Type unions may widen the input set, never narrow it.** Pyspark
49+
accepts `Column` or `str` (column name) for most args; we accept
50+
`Expr` already, and widening to `Expr | int` / `Expr | str` for
51+
literal-friendly arguments is on-brand because the int/str case is
52+
exactly what a pyspark caller would also try. Just verify the widened
53+
set is a superset of what pyspark accepts for that arg.
54+
- **Extra keyword arguments are allowed** as long as they default to
55+
`None` and pyspark's positional/keyword form still works (e.g. the
56+
spark `avg`/`try_sum`/`collect_list`/`collect_set` retain DataFusion's
57+
`distinct`/`filter`/`order_by`/`null_treatment` kwargs).
58+
59+
Practical effect: in `functions.spark`, apply Categories A and (where
60+
pyspark exposes the same arg as a non-`Expr`) B normally, but cross-check
61+
each proposed signature against `pyspark.sql.functions` before landing
62+
it. When pyspark's own type hint is `Column | str` for a "column name"
63+
arg, prefer leaving the spark wrapper at `Expr` — Category C
64+
("`Expr | str` meaning column name") is unusual in `functions.py` and
65+
should remain so in `functions.spark`.
5266

5367
## How to Identify Candidates
5468

55-
The user may specify a scope via `$ARGUMENTS`. If no scope is given or "all" is specified, audit all functions in `python/datafusion/functions/__init__.py`.
69+
The user may specify a scope via `$ARGUMENTS`. If no scope is given or "all" is specified, audit all functions in `python/datafusion/functions/__init__.py` **and** `python/datafusion/functions/spark.py`. When updating a spark-namespace function, apply the compatibility rules from "Scope" above on top of the standard analysis.
5670

5771
For each function, determine if any parameter can accept native Python types by evaluating **two complementary signals**:
5872

0 commit comments

Comments
 (0)