Skip to content

feat: strict typing stream.iter_array - #1993

Open
FBruzzesi wants to merge 2 commits into
online-ml:mainfrom
FBruzzesi:feat/typing-iter-array
Open

feat: strict typing stream.iter_array#1993
FBruzzesi wants to merge 2 commits into
online-ml:mainfrom
FBruzzesi:feat/typing-iter-array

Conversation

@FBruzzesi

Copy link
Copy Markdown
Contributor

Scope as per title: add strict typing for stream.iter_array module/function.

This is the most tricky typing PR I did so far in river as the runtime behavior allows for a lot of cases.


@FBruzzesi

Copy link
Copy Markdown
Contributor Author

Gentle ping to @e10e3 and @mugulmd for reviewing 🙏🏼

@codspeed-hq

codspeed-hq Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 98 untouched benchmarks
⏩ 16 skipped benchmarks1


Comparing FBruzzesi:feat/typing-iter-array (92863dc) with main (fe248b4)

Open in CodSpeed

Footnotes

  1. 16 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@e10e3 e10e3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the effort, that's indeed a lot of overloads for that function.

I'm surprised by the quantity of changes that you propose. Usually, typing PRs only add types, they aim at not changing the behaviour. If they make changes, it's as little as possible, only to improve the quality of the type annotations.

Hence, why did you need to add tests in a typing PR?
Why did you need to change the whole implementation of iter_array?

"""The type of a single target value, i.e. of a row of a 1D array of targets."""

Array: typing.TypeAlias = "np.ndarray | Sequence[typing.Any]"
"""A numpy array or a plain Python sequence."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment is redundant.

"""One row of features, labeled, which is what an estimator takes as its `x`."""


def _passthrough(row: str) -> str:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is used exactly once. Why not define it at its usage location?

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