fix(python): Correct type hint for map_columns function parameter#26487
Open
veeceey wants to merge 1 commit intopola-rs:mainfrom
Open
fix(python): Correct type hint for map_columns function parameter#26487veeceey wants to merge 1 commit intopola-rs:mainfrom
map_columns function parameter#26487veeceey wants to merge 1 commit intopola-rs:mainfrom
Conversation
The type hint for the `function` parameter in `DataFrame.map_columns` was `Callable[[Series], Series]`, which incorrectly implied the function only accepts a single Series argument. However, the implementation passes `*args` and `**kwargs` to the function. This updates the type hint to `Callable[Concatenate[Series, P], Series]` using ParamSpec to accurately reflect that the function can accept additional positional and keyword arguments after the Series parameter. Fixes pola-rs#26371
Author
Manual Test ResultsI've verified the fix works correctly with various test cases: import polars as pl
def function_with_args(s: pl.Series, a: int, b: int = 0) -> pl.Series:
"""Function that takes Series plus additional positional and keyword arguments."""
return a * s + b
def simple_function(s: pl.Series) -> pl.Series:
"""Function that only takes a Series (original functionality)."""
return s * 2
# Test 1: Function with additional positional and keyword arguments
df = pl.DataFrame({"a": [1, 2]})
result1 = df.map_columns("a", function_with_args, 1, b=2)
# Test 2: Multiple columns with the same function
df2 = pl.DataFrame({"a": [1, 2], "b": [3, 4]})
result2 = df2.map_columns(["a", "b"], function_with_args, 2, b=1)
# Test 3: Simple function without extra args (backward compatibility)
result3 = df.map_columns("a", simple_function)
# Test 4: Lambda with extra args
result4 = df.map_columns("a", lambda s, mult: s * mult, 3)All tests execute successfully and produce correct results. The new type hints properly support:
Type checkers (mypy/pyright) will now correctly validate that custom functions passed to |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26487 +/- ##
==========================================
- Coverage 81.08% 80.83% -0.25%
==========================================
Files 1783 1783
Lines 243288 243288
Branches 3074 3074
==========================================
- Hits 197263 196668 -595
- Misses 45222 45817 +595
Partials 803 803 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
map_columns function parameter
Author
|
Friendly ping - any chance someone could take a look at this when they get a chance? Happy to make any changes if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #26371
The type hint for the
functionparameter inDataFrame.map_columnswasCallable[[Series], Series], which incorrectly implied the function only accepts a singleSeriesargument. However, the implementation passes*argsand**kwargsto the function (line 6964).This PR updates the type hint to
Callable[Concatenate[Series, P], Series]usingParamSpecto accurately reflect that the function can accept additional positional and keyword arguments after theSeriesparameter.Changes
functionparameter type hint fromCallable[[Series], Series]toCallable[Concatenate[Series, P], Series]*argstype hint fromAnytoP.args**kwargstype hint fromAnytoP.kwargsParamSpecPwas already defined in the TYPE_CHECKING block (line 206)Test Plan
Created and ran a test script demonstrating the fix:
The code executes correctly and type checkers can now properly validate that functions passed to
map_columnscan accept additional arguments beyond the Series parameter.