Repository navigation
Document the real default of index_col in train_test_split_pyspark - #1646
Conversation
train_test_split_pyspark takes index_col="tmp_index_col", but its docstring says "Default None." The other four arguments in the same docstring all state their real defaults, and model.py documents the same argument correctly as "tmp_index_col" in several places, so this one line was the outlier. The value matters because index_col is forwarded to psDataFrame.to_spark(index_col=...), where None drops the index instead of writing it to a column.
There was a problem hiding this comment.
🟢 Approval recommended
The documentation-only correction matches the function’s default and has no unresolved findings.
No open issues 🎉
What changed in this PR
Corrects the documented default for index_col in train_test_split_pyspark.
Changes:
- Replaces
Nonewith"tmp_index_col"in the docstring; runtime behavior is unchanged.
| File | Description |
|---|---|
| flaml/automl/spark/utils.py | Documents the actual index_col default. |
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Reviewed 81323ad27c5322b43a646eb60f874e32ee873c5e with review-code (Low, gpt-6.1-sol): no actionable source finding in the inspected scope.
The corrected index_col description now matches the actual "tmp_index_col" signature default and its forwarding to Spark/pandas-on-Spark conversion APIs. The complete executable module AST is identical after removing only this changed function docstring; no runtime behavior or default changed.
No Spark runtime test was executed for this documentation-only correction. The current Balanced Copilot recommendation was checked separately from the actual workflow results.
Approval remains withheld: applicable CI check contexts are not all successful; actual current-head pull_request workflows are not all successful. The current-head Build workflow failed on four Ubuntu Python variants. Its representative Ubuntu 3.13 log reports the pytest step timed out after 120 minutes; causes for the other jobs were not individually established. A clean source review or passing local selection does not replace successful actual CI. Failed workflows were not rerun.
Posted by thinkall-agent-auto-reviewer
|
Thanks for the quick look. Flagging that the four Ubuntu is the only leg that installs pyspark and prophet, so it carries the spark suite. Is that 120-minute ceiling normally close to the wire, or does something look newly slow to you? Happy to rebase once it clears if that helps. |
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Re-reviewed commits 81323ad27c5322b43a646eb60f874e32ee873c5e -> b9873fbfba00c140d94d3e4a84b7a61ec0b1fe76. The only delta is main's website/yarn.lock update of follow-redirects 1.16.0 -> 1.16.1. One explicitly head-bound review-code Low execution (actual runtime gpt-6.1-sol) found no new issue in this identical inherited patch. Controller Git comparisons verify all non-lockfile tree entries are unchanged. This is a scoped merge-delta review, not a fresh clean signoff for unchanged feature code.
Version/URL/caret/SRI shape checks pass. No independent local registry/tarball verification, package smoke test or website rebuild is claimed. Previous feature findings and runtime validation limits remain preserved.
At the 2026-10-26T12:13:44.943946+00:00 gate snapshot, actual current-head workflows were: Code formatting completed/success; docs completed/success; Build in_progress/no conclusion. Copilot has explicit Balanced Approval recommended; 0 review thread(s) remain unresolved. Gate approval is not successful CI completion, and no new PR approval is being submitted while required gates remain unmet.
Posted by thinkall-agent-auto-reviewer
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Overall review of the whole PR at b9873fbfba00c140d94d3e4a84b7a61ec0b1fe76: approved.
The complete feature diff is byte-identical to the prior independent clean review: the index_col docstring now matches the actual "tmp_index_col" default. Executable behavior is unchanged. The current head is also explicitly bound in the clean inherited-main review. I reused the verified review-code Low evidence (actual runtime gpt-6.1-sol) rather than rerunning an unchanged source review. No new local Spark execution is claimed.
Immediately before submission, I verified the same current open, non-draft, non-self head, all review threads resolved, explicit current-head Balanced Copilot Approval recommended (review), and successful applicable check contexts. I also queried the actual Actions workflow runs directly: Code formatting succeeded; Build succeeded; docs succeeded. The deployment-only gh-release job is intentionally skipped for pull requests by its explicit event guard; it is not being counted as a passed validation.
Posted by thinkall-agent-auto-reviewer
train_test_split_pysparktakesindex_col="tmp_index_col", but its docstring saysDefault None.The other four arguments in the same docstring all state their real defaults, and
flaml/automl/model.pydocuments the same argument correctly asDefault to "tmp_index_col".in several places, so this line was the outlier. The value is not cosmetic —index_colis forwarded topsDataFrame.to_spark(index_col=...), whereNonedrops the index rather than writing it to a column.Docstring only, no behaviour change.