Conversation
|
LGTM |
|
|
||
|
|
||
| cdef floating euclidian_dist(floating* a, floating* b, int n_features) nogil: | ||
| cdef floating euclidean_dist(floating* a, floating* b, int n_features) nogil: |
There was a problem hiding this comment.
is this function considered private or public?...
There was a problem hiding this comment.
It looks private. The outputs of finding all files containing euclidian_dist are as follows:
$ grep -r "euclidian_dist(" .
./sklearn/cluster/_k_means_elkan.pyx:cdef floating euclidian_dist(floating* a, floating* b, int n_features) nogil:
./sklearn/cluster/_k_means_elkan.pyx: d_c = euclidian_dist(x, centers, n_features)
./sklearn/cluster/_k_means_elkan.pyx: dist = euclidian_dist(x, c, n_features)
./sklearn/cluster/_k_means_elkan.pyx: upper_bound = euclidian_dist(x_p, centers_p + label * n_features, n_features)
./sklearn/cluster/_k_means_elkan.pyx: distance = euclidian_dist(x_p, centers_p + center_index * n_features, n_features)
There was a problem hiding this comment.
for cdef functions, I believe if there is no pxd it's basically private, because it cannot be imported from third party projects. @ogrisel / @amueller / @GaelVaroquaux am I right?
Maybe we should make it explicit by adding an underscore too.
There was a problem hiding this comment.
for cdef functions, I believe if there is no pxd it's basically private
Not a cython expert, but I think you are right.
There was a problem hiding this comment.
In [1]: from sklearn.cluster import _k_means_elkan
In [2]: _k_means_elkan.euclidi<TAB> # nothing is foundThere was a problem hiding this comment.
@lesteve that's what I thought. should @taehoonlee then prepend an underscore to make it clear to us in the future?
There was a problem hiding this comment.
Not sure, but I would just keep it like this.
|
Let's merge this one, thanks @taehoonlee ! |
|
for cdef functions, I believe if there is no pxd it's basically private
Not a cython expert, but I think you are right.
Yes!
|
No description provided.