Please make sure the test is not too long, reducing N and T if necessary.
You can do it with pytest --duration 10 sklearn/neighbors/tests/test_kde.py::test_kde_sample_weights.

Please avoid using single-letter variables (except X).
example: change N to n_samples.

W = rng.randint(10, size=N) is probably clearer.

Indeed, however the goal is not to have weights uniformly distributed, since this would be (asymptotically) equivalent to having uniform weights.
So I picked weights to be entirely determined the (L1) norm of the vector to have a simple pattern where weights are positive integers.

Why T // d ?

We want the first argument (the number of test points) to be an integer, both in python2 and python3.

Yes but why would you divide T by d? It is just another integer that you could name n_samples_2, isn't it?

Yes you're right, I'll rename this to make it more expressive.
I do this mainly because I want to keep the number of operations under control while increasing the dimension of the space, so I decrease the cardinality of the test sample.

sample_weight.ndim is more appropriate.

Why do you need both Z and sum_weight?

Z is either sum_weight or n_samples, so in this context sum_weight is just as useful as n_samples although much costlier to produce (hence better to compute it just once at __init__ time).

Yes, but why not store sum_weight = n_samples in init and use it in all cases here rather than introduce a new variable

That would make sense indeed, thanks for the suggestion !

Does all this branching provide much benefit?

I did not compare it to

                   for i in range(node_info.idx_start, node_info.idx_end):
                       dist_pt = self.dist(pt, data + n_features * idx_array[i],
                                            n_features)
                        log_density = compute_log_kernel(dist_pt, h, kernel)
                        if with_sample_weight: 
                            log_weight = np.log(sample_weight[idx_array[i]])
                        else:
                            log_weight = 0
                        global_log_min_bound = logaddexp(global_log_min_bound,
                                                                 log_density + log_weight)

but that did not look much simpler to me. Do you have another suggestion that I am missing ?

It looks more maintainable to me, which is my concern. Near-duplicate code is hard to see differences in

I see your point, I'll do some benchmarking to see if this branching is a actually improving anything.

Removing the branching slows down the fit method (both with and without sample weights unfortunately) by about 10-15%, for 10^5 sample with 20 columns on my local machine.

I committed the simplified code anyway. Happy to revert the commit if we want to avoid the consequent slowdown.

Usually we use check_consistent_length

Thanks !

How long does this test take to run? This seems larger than necessary to prove the point

5 seconds on my laptop. Indeed, I can decrease this to a few 100s and this would take ~1 sec.

np.repeat should work here

Thanks !

Any reason to believe minkowski without parameter would work differently?

Why not use check_array?

Done

Are these covered by tests?

Added !

I think we'd usually not want this extra indentation. You could do:

(log_dens_contribution +
 log_weight)

Please create a cdef inline to avoid repetition of this idiom.

This is not covered by the tests

Bien vu ! Added a test for that as well.