-
Notifications
You must be signed in to change notification settings - Fork 166
feat: Add support for nw.int_range
for eager backends
#2895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Hey @FBruzzesi, as I've mentioned 8347839 times, I'm very keen to see this in Would you be okay if we let this one marinate for a bit and maybe target the release after the next? π |
π
Yeah I was not expecting this to be finalized by tomorrow. It has some sharp edges that need some love and work |
@MarcoGorelli any "trick" to avoid the custom check in precommit?
I thought about emulating that |
You could add this to IntegerDType: TypeAlias = "dtypes.IntegerType | type[dtypes.IntegerType]" |
#2982 removed from everywhere else
> TypeError: argument 'step': 'PolarsExpr' object cannot be interpreted as an integer https://github.com/narwhals-dev/narwhals/actions/runs/17499840563/job/49709898155?pr=2895
@MarcoGorelli pinging again as (#2895 (comment)) was a month ago π«£ |
What type of PR is this? (check all applicable)
Checklist
If you have comments or can explain your changes, please do so below
Related issue: #2722
I am slightly concerned that this could turn out to be a month long PR. I will open it as draft for now. There are a couple of pain points I already know:
I was very careful with type hints, yet it somehow doesn't pass type checkerFixedeager: bool
. In our case it's a bit more complex than that. Instead of adding yet another argument to the function, I allowed foreager
to be False (default), None or the backend/implementation that should back the series.