[libcamera-devel,v1,1/3] utils: raspberrypi: ctt: Fix namespace for sklearn NearestCentroid function

13175 diff mbox series

The NearestCentroid function is in the sklearn.neighbors namespace
since version 0.22.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
 utils/raspberrypi/ctt/ctt_tools.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Hi David,

Thank you for the patch.

On Mon, Aug 02, 2021 at 09:20:34AM +0100, David Plowman wrote:
> The NearestCentroid function is in the sklearn.neighbors namespace
> since version 0.22.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  utils/raspberrypi/ctt/ctt_tools.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py
> index 48e0aac2..41c4efb6 100644
> --- a/utils/raspberrypi/ctt/ctt_tools.py
> +++ b/utils/raspberrypi/ctt/ctt_tools.py
> @@ -14,7 +14,12 @@ import imutils
>  import sys
>  import matplotlib.pyplot as plt
>  from sklearn import cluster as cluster
> -from sklearn.neighbors.nearest_centroid import NearestCentroid as get_centroids
> +try:
> +    # From sklearn version 0.22 onwards, NearestCentroid is no longer in the
> +    # nearest_centroid module
> +    from sklearn.neighbors.nearest_centroid import NearestCentroid as get_centroids
> +except (ModuleNotFoundError):
> +    from sklearn.neighbors import NearestCentroid as get_centroids

Actually, NearestCentroid has always been available directly from
sklearn.neighbors. The change in 0.22 is that it's not available from
sklearn.neighbors.nearest_centroid anymore. I've just tested importing

from sklearn.neighbors import NearestCentroid as get_centroids

with 0.21.0, and it works fine. Sorry for the misleading review of the
previous version. The code was fine, it's just the commit message that
needed updating. If you're fine with the following, I can update when
applying.

"Starting in version 0.22, the NearestCentroid function is only
available in the sklearn.neighbors namespace, when it was previously
available in both the sklearn.neighbors.nearest_centroid and
sklearn.neighbors namespaces. Use sklearn.neighbors as it works on all
versions of sklearn."

>  
>  """
>  This file contains some useful tools, the details of which aren't important to

Hi Laurent

What a confusing story, it was perhaps a bit unlucky that our original
version was the one that was going to break! But thank you very much
for the suggestion - if you could do that it would be greatly
appreciated.

Thanks!
David

On Mon, 2 Aug 2021 at 18:41, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Mon, Aug 02, 2021 at 09:20:34AM +0100, David Plowman wrote:
> > The NearestCentroid function is in the sklearn.neighbors namespace
> > since version 0.22.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  utils/raspberrypi/ctt/ctt_tools.py | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py
> > index 48e0aac2..41c4efb6 100644
> > --- a/utils/raspberrypi/ctt/ctt_tools.py
> > +++ b/utils/raspberrypi/ctt/ctt_tools.py
> > @@ -14,7 +14,12 @@ import imutils
> >  import sys
> >  import matplotlib.pyplot as plt
> >  from sklearn import cluster as cluster
> > -from sklearn.neighbors.nearest_centroid import NearestCentroid as get_centroids
> > +try:
> > +    # From sklearn version 0.22 onwards, NearestCentroid is no longer in the
> > +    # nearest_centroid module
> > +    from sklearn.neighbors.nearest_centroid import NearestCentroid as get_centroids
> > +except (ModuleNotFoundError):
> > +    from sklearn.neighbors import NearestCentroid as get_centroids
>
> Actually, NearestCentroid has always been available directly from
> sklearn.neighbors. The change in 0.22 is that it's not available from
> sklearn.neighbors.nearest_centroid anymore. I've just tested importing
>
> from sklearn.neighbors import NearestCentroid as get_centroids
>
> with 0.21.0, and it works fine. Sorry for the misleading review of the
> previous version. The code was fine, it's just the commit message that
> needed updating. If you're fine with the following, I can update when
> applying.
>
> "Starting in version 0.22, the NearestCentroid function is only
> available in the sklearn.neighbors namespace, when it was previously
> available in both the sklearn.neighbors.nearest_centroid and
> sklearn.neighbors namespaces. Use sklearn.neighbors as it works on all
> versions of sklearn."
>
> >
> >  """
> >  This file contains some useful tools, the details of which aren't important to
>
> --
> Regards,
>
> Laurent Pinchart