Skip to content

Fix KDTree scalar return when max_num_neighbours=1#90

Open
Aadrit555 wants to merge 10 commits intomllam:mainfrom
Aadrit555:fix/kdtree-k1-scalar
Open

Fix KDTree scalar return when max_num_neighbours=1#90
Aadrit555 wants to merge 10 commits intomllam:mainfrom
Aadrit555:fix/kdtree-k1-scalar

Conversation

@Aadrit555
Copy link
Copy Markdown

@Aadrit555 Aadrit555 commented Mar 4, 2026

Changes made -

I fixed a crash in connect_nodes_across_graphs when called with method="nearest_neighbours" and max_num_neighbours=1.

Root cause: scipy.spatial.KDTree.query() returns different types depending on the value of k:

  • k = 1 → scalar (numpy.int64)
  • k > 1numpy.ndarray

The function _find_neighbour_node_idxs_in_source_mesh returned the raw query result, which the caller then iterated over. When k=1, iterating over a scalar raises:

TypeError: 'numpy.int64' object is not iterable

Fix: Wrap the return value with np.atleast_1d() so the result is always iterable, regardless of k.

# BEFORE
def _find_neighbour_node_idxs_in_source_mesh(xy_target):
    neigh_idxs = kdt_s.query(xy_target, max_num_neighbours)[1]
    return neigh_idxs

# AFTER
def _find_neighbour_node_idxs_in_source_mesh(xy_target):
    neigh_idxs = kdt_s.query(xy_target, max_num_neighbours)[1]
    # KDTree.query returns a scalar when k=1 and a 1-D array when k>1.
    # np.atleast_1d normalises both cases so the caller can always iterate.
    return np.atleast_1d(neigh_idxs)

No dependencies are required for this change.

Issue Link

Closes #83

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📖 Documentation (Addition or improvements to documentation)

Checklist before requesting a review

  • My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
  • I have performed a self-review of my code
  • For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
  • I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
  • I have updated the documentation to cover introduced code changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have given the PR a name that clearly describes the change, written in imperative form (context).
  • I have requested a reviewer and an assignee (assignee is responsible for merging)

Checklist for reviewers

Each PR comes with its own improvements and flaws. The reviewer should check the following:

  • the code is readable
  • the code is well tested
  • the code is documented (including return types and parameters)
  • the code is easy to maintain

Author checklist after completed review

  • I have added a line to the CHANGELOG describing this change, in a section
    reflecting type of change (add section where missing):
    • fixes: connect_nodes_across_graphs crash when max_num_neighbours=1 due to KDTree.query returning a scalar for k=1

Checklist for assignee

  • PR is up to date with the base branch
  • the tests pass
  • author has added an entry to the changelog (and designated the change as added, changed or fixed)
  • Once the PR is ready to be merged, squash commits and merge the PR.

Copy link
Copy Markdown
Member

@leifdenby leifdenby left a comment

Choose a reason for hiding this comment

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

Looking good. Please add a changelog entry :)

@Aadrit555
Copy link
Copy Markdown
Author

@leifdenby did it can you check please, thanks.

@leifdenby
Copy link
Copy Markdown
Member

Can you fix the linting errors please? Just run pre-commit locally before re-committing

@Aadrit555
Copy link
Copy Markdown
Author

@leifdenby Sorry for the messy commits — being fairly new to open source I got a bit confused while trying to fix the linting errors. I've got all pre-commit checks passing locally now. I tried to squash the commits via rebase but would need your help to do that since I don't have the required permissions. Happy to clean it up however works best for you!

@Aadrit555 Aadrit555 requested a review from leifdenby March 7, 2026 09:02
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.

Bug: nearest_neighbours crashes when max_num_neighbours=1

2 participants