Skip to content

feat(typing): make types more precise for many common Values#11771

Merged
cpcloud merged 1 commit intoibis-project:mainfrom
NickCrews:typing-improvements
Feb 4, 2026
Merged

feat(typing): make types more precise for many common Values#11771
cpcloud merged 1 commit intoibis-project:mainfrom
NickCrews:typing-improvements

Conversation

@NickCrews
Copy link
Copy Markdown
Contributor

@NickCrews NickCrews commented Nov 20, 2025

This partially mitigates #11680, at least for some of the more common types.

In particular, it solves this ask:
#11680 (comment)

I focused on the most common use cases, eg StringValue + "foo" is more important than NumericColumn.kurtosis()

This fixes several sorts of problems:

  • It types methods as returning the right datashape. Sometimes this was as simple as use Self, but often this required @overload because the method depends on the datashape of second args, or it is an eg string->int method
  • It make datatypes more precise in some places, eg before it was simply returning NumericValue, but we actually know it is FloatingScalar
  • It fixes the typing in some places. eg in StringValue.__getitem__, it actually accepts IntegerColumn as well, not just IntegerScalar, as it was typed before. Or IntegerValue.convert_base() incorrectly said it returned a IntegerValue, but it actually returns a StringValue

Copy link
Copy Markdown
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there's something I'm missing here I don't see how return-type-only overloads are actually useful.

return ibis.dense_rank().over(order_by=self)

def percent_rank(self) -> Column:
def percent_rank(self) -> ir.IntegerColumn:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this guaranteed to be an IntegerColumn. Consider the example in the docstring.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks you're right

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted to ir.FloatingColumn

self: ir.BooleanScalar, true_expr: ir.Scalar, false_expr: ir.Scalar, /
) -> ir.Scalar: ...
@overload
def ifelse(self, true_expr: ir.Value, false_expr: ir.Value, /) -> ir.Column: ...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need overloads for these? You can't use overload for return values anyway. Consider this example:

@overload
def foo(x: int) -> str: ...
@overload
def foo(x: int) -> int: ...

y = foo(1)  # which overload is called?

There's no straightforward way to compute the type of y.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was going for was more of

@overload
def foo(x: SubtypeOfInt) -> str: ...
@overload
def foo(x: int) -> int: ...

so that the first overload would be selected as the first matching case. I can do some more tests though to ensure that this is actually happening.

@cpcloud
Copy link
Copy Markdown
Member

cpcloud commented Jan 25, 2026

@NickCrews Can't you split the changes that make return types more precise out into a separate PR that doesn't contain the changes with all the additional overloads?

@NickCrews
Copy link
Copy Markdown
Contributor Author

Yes!

@NickCrews NickCrews force-pushed the typing-improvements branch from 3288afd to d38ac20 Compare January 25, 2026 19:32
@NickCrews
Copy link
Copy Markdown
Contributor Author

OK in that new version I removed all overloads, but I did go through even more methods and made them more correct and accepting of plain python values and Deferreds, eg adjusting where:ir.BooleanValue | None = None to where:bool | ir.BooleanValue | Deferred | None = None

@NickCrews NickCrews force-pushed the typing-improvements branch from d38ac20 to 685d725 Compare January 28, 2026 20:18
@NickCrews NickCrews force-pushed the typing-improvements branch from 685d725 to 0f9c078 Compare February 4, 2026 05:36
Copy link
Copy Markdown
Member

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@cpcloud cpcloud merged commit 2eb772a into ibis-project:main Feb 4, 2026
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants