Skip to content

Strict typing: neighbors sub-package - #2007

Open
mugulmd wants to merge 5 commits into
online-ml:mainfrom
mugulmd:typing-neighbors
Open

mugulmd wants to merge 5 commits into
online-ml:mainfrom
mugulmd:typing-neighbors

Conversation

@mugulmd

@mugulmd mugulmd commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Description

Related to #1944

Implement strict typing for the river.neighbors package as it only depends on already strictly typed packages.

Notes

In SWINN the _data field was typed as sequence of Vertex | None however the None value was only used to handle the "vertex replacement" occurring when adding a new vertex on a full sliding window. Using this None intermediate value does not bring anything to the table, since the vertex being replaced is being isolated first.
Therefore, in order to simplify the typing of the SWINN class and make the code more readable we made the _data field a sequence of Vertex only.

Next steps

Currently all items considered for nearest neighbors queries are simply typed as Any, which makes things simple considering the potential variety of item types to handle. However using generics instead would provide much more precise typing information. This would mean changing quite a lot in the package's interface, so it is left for later.

@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 100 untouched benchmarks
⏩ 16 skipped benchmarks1


Comparing mugulmd:typing-neighbors (d75cc3e) with main (71d86ee)

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.

@mugulmd
mugulmd marked this pull request as ready for review September 17, 2026 16:10
if len(self) == self.maxlen:
neighbors, dists = self._search(node.item, self.graph_k, exclude={node.uuid})
# neighbors, dists = self._search(node.item, self.graph_k, exclude={node.uuid})
neighbors, dists = self._search(node.item, self.graph_k)

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.

Isn't this a logic change?

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