Repository navigation
Document the real default of index_col in train_test_split_pyspark - #1646
Venish Paneliya (VenishPaneliya) wants to merge 1 commit into
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
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. |
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.