Fix call to NearestNeighbors.kneighbors() without args.#6333
Conversation
csadorf
added
bug
csadorf
force-pushed
the
fix-nearest-neighbors-kneighbors-call-without-args
branch
from
ff0a5d9 to
02d16e9
Compare
|
Should we add the pytest from #6317? |
|
This looks nice to me, though I have to admit that I could not explain to you why this works/if this is what the creators of the decorators wanted us to do. But that is more a comment about the state that the decorator stuff is in than this PR. |
|
I realized that my implementation was in fact breaking (as surfaced by the failing doc-string example test) so decided to adopt and build on top of @betatim 's solution after all. After careful review and some testing I believe that this revision should be safe to introduce and does not break current behavior. |
| # Check for default name in input args | ||
| if arg_name in sig.parameters: | ||
| return arg_name, params.index(arg_name) | ||
| param = sig.parameters[arg_name] |
There was a problem hiding this comment.
Can't remember off the top of my head, but the refactor of decorators you are doing removes this auto inspection of signature, no? Just wondering since it'd be nice to not have magic behavior of this.
There was a problem hiding this comment.
Can you explain a bit more? The code here looks more or less the same, so maybe you mean something that effects something somewhere else because of this change?
There was a problem hiding this comment.
@dantegd I am a big confused by your question as well. Yes, the infrastructure revision will eliminate much of the magic and guess work, but here I am just inspecting the signature similar like before, but account for a provided default argument similar to how @betatim structured this in the earlier PR.
Signed-off-by: Simon Adorf <sadorf@nvidia.com>
csadorf
force-pushed
the
fix-nearest-neighbors-kneighbors-call-without-args
branch
from
503f415 to
d16508a
Compare
|
/merge |
csadorf
deleted the
fix-nearest-neighbors-kneighbors-call-without-args
branch
PRs being backported: - [x] #6234 - [x] #6306 - [x] #6320 - [x] #6319 - [x] #6327 - [x] #6333 - [x] #6142 - [x] #6223 - [x] #6235 - [x] #6317 - [x] #6331 - [x] #6326 - [x] #6332 - [x] #6347 - [x] #6348 - [x] #6337 - [x] #6355 - [x] #6354 - [x] #6322 - [x] #6353 - [x] #6359 - [x] #6364 - [x] #6363 - [x] [FIL BATCH_TREE_REORG fix for SM90, 100 and 120](a3e419a) --------- Co-authored-by: William Hicks <whicks@nvidia.com>
4 participants
Alternative solution for #6317 .