| Message ID | 20210721115220.5090-2-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for fixing this! On Wed, 21 Jul 2021 at 12:52, David Plowman <david.plowman@raspberrypi.com> wrote: > The NearestCentroid function is now in the sklearn.neighbors > namespace. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > utils/raspberrypi/ctt/ctt_tools.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utils/raspberrypi/ctt/ctt_tools.py > b/utils/raspberrypi/ctt/ctt_tools.py > index 48e0aac2..8728ff16 100644 > --- a/utils/raspberrypi/ctt/ctt_tools.py > +++ b/utils/raspberrypi/ctt/ctt_tools.py > @@ -14,7 +14,7 @@ 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 > +from sklearn.neighbors import NearestCentroid as get_centroids > > """ > This file contains some useful tools, the details of which aren't > important to > -- > 2.20.1 > >
Hi David, On 21/07/2021 12:52, David Plowman wrote: > The NearestCentroid function is now in the sklearn.neighbors > namespace. > Does this have requirements of a specific version of the external import? It would probably be helpful to mention the version that it became required or ensure that the newest version is now a required version somehow. But otherwise, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > utils/raspberrypi/ctt/ctt_tools.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py > index 48e0aac2..8728ff16 100644 > --- a/utils/raspberrypi/ctt/ctt_tools.py > +++ b/utils/raspberrypi/ctt/ctt_tools.py > @@ -14,7 +14,7 @@ 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 > +from sklearn.neighbors import NearestCentroid as get_centroids > > """ > This file contains some useful tools, the details of which aren't important to >
Hi Kieran I did go looking for the version when this changed, but even trawling through the release notes didn't really enlighten me. I found a commit here (https://github.com/scikit-learn/scikit-learn/commit/62aee0666e8803f20ecf0f6214621367e50f3961#diff-9f63dcf8f128db106a66cc33db9816f91db51e286f5f13af3f87405468b3df2b) that clearly has some bearing on this, from back in October 2019, but there doesn't seem to be anything older there. So unless anyone else can enlighten me, I've thrown in the towel... :( Thanks anyway! David On Mon, 26 Jul 2021 at 11:31, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi David, > > On 21/07/2021 12:52, David Plowman wrote: > > The NearestCentroid function is now in the sklearn.neighbors > > namespace. > > > > Does this have requirements of a specific version of the external > import? It would probably be helpful to mention the version that it > became required or ensure that the newest version is now a required > version somehow. > > But otherwise, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > --- > > utils/raspberrypi/ctt/ctt_tools.py | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py > > index 48e0aac2..8728ff16 100644 > > --- a/utils/raspberrypi/ctt/ctt_tools.py > > +++ b/utils/raspberrypi/ctt/ctt_tools.py > > @@ -14,7 +14,7 @@ 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 > > +from sklearn.neighbors import NearestCentroid as get_centroids > > > > """ > > This file contains some useful tools, the details of which aren't important to > >
Hi David, On Wed, Jul 28, 2021 at 09:27:49AM +0100, David Plowman wrote: > Hi Kieran > > I did go looking for the version when this changed, but even trawling > through the release notes didn't really enlighten me. I found a commit > here (https://github.com/scikit-learn/scikit-learn/commit/62aee0666e8803f20ecf0f6214621367e50f3961#diff-9f63dcf8f128db106a66cc33db9816f91db51e286f5f13af3f87405468b3df2b) > that clearly has some bearing on this, from back in October 2019, but > there doesn't seem to be anything older there. So unless anyone else > can enlighten me, I've thrown in the towel... :( $ git tag --contains 62aee0666e8803f20ecf0f6214621367e50f3961 0.22 0.22.1 0.22.2 0.22.2.post1 0.22rc1 0.22rc2 0.22rc2.post1 0.22rc3 0.23.0 0.23.0rc1 0.23.1 0.23.2 0.24.0 0.24.0rc1 0.24.1 0.24.2 I think we can reasonably conclude that the change appeared in 0.22 :-) Changing the API like this isn't nice :-S Is there value in trying to preserve backward compatibility ? If that's of any interest, Debian ships 0.20.2 in the latest stable, while Ubuntu started shipping 0.22.2 in 20.04LTS. If backward compatibility isn't worth it, we should at least document the minimal version somewhere (a README.rst in utils/raspberrypi/ctt/ maybe ?). > On Mon, 26 Jul 2021 at 11:31, Kieran Bingham wrote: > > On 21/07/2021 12:52, David Plowman wrote: > > > The NearestCentroid function is now in the sklearn.neighbors > > > namespace. > > > > Does this have requirements of a specific version of the external > > import? It would probably be helpful to mention the version that it > > became required or ensure that the newest version is now a required > > version somehow. > > > > But otherwise, > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > --- > > > utils/raspberrypi/ctt/ctt_tools.py | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py > > > index 48e0aac2..8728ff16 100644 > > > --- a/utils/raspberrypi/ctt/ctt_tools.py > > > +++ b/utils/raspberrypi/ctt/ctt_tools.py > > > @@ -14,7 +14,7 @@ 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 > > > +from sklearn.neighbors import NearestCentroid as get_centroids > > > > > > """ > > > This file contains some useful tools, the details of which aren't important to
Hi Laurent Thanks for the info, didn't know I could do that! Given that our documentation for tuning is all elsewhere, maybe it's easier just to handle the change in the code. Let me submit a new version of this commit which does that! Thanks David On Mon, 2 Aug 2021 at 01:39, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi David, > > On Wed, Jul 28, 2021 at 09:27:49AM +0100, David Plowman wrote: > > Hi Kieran > > > > I did go looking for the version when this changed, but even trawling > > through the release notes didn't really enlighten me. I found a commit > > here (https://github.com/scikit-learn/scikit-learn/commit/62aee0666e8803f20ecf0f6214621367e50f3961#diff-9f63dcf8f128db106a66cc33db9816f91db51e286f5f13af3f87405468b3df2b) > > that clearly has some bearing on this, from back in October 2019, but > > there doesn't seem to be anything older there. So unless anyone else > > can enlighten me, I've thrown in the towel... :( > > $ git tag --contains 62aee0666e8803f20ecf0f6214621367e50f3961 > 0.22 > 0.22.1 > 0.22.2 > 0.22.2.post1 > 0.22rc1 > 0.22rc2 > 0.22rc2.post1 > 0.22rc3 > 0.23.0 > 0.23.0rc1 > 0.23.1 > 0.23.2 > 0.24.0 > 0.24.0rc1 > 0.24.1 > 0.24.2 > > I think we can reasonably conclude that the change appeared in 0.22 :-) > Changing the API like this isn't nice :-S > > Is there value in trying to preserve backward compatibility ? If that's > of any interest, Debian ships 0.20.2 in the latest stable, while Ubuntu > started shipping 0.22.2 in 20.04LTS. > > If backward compatibility isn't worth it, we should at least document > the minimal version somewhere (a README.rst in utils/raspberrypi/ctt/ > maybe ?). > > > On Mon, 26 Jul 2021 at 11:31, Kieran Bingham wrote: > > > On 21/07/2021 12:52, David Plowman wrote: > > > > The NearestCentroid function is now in the sklearn.neighbors > > > > namespace. > > > > > > Does this have requirements of a specific version of the external > > > import? It would probably be helpful to mention the version that it > > > became required or ensure that the newest version is now a required > > > version somehow. > > > > > > But otherwise, > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > --- > > > > utils/raspberrypi/ctt/ctt_tools.py | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py > > > > index 48e0aac2..8728ff16 100644 > > > > --- a/utils/raspberrypi/ctt/ctt_tools.py > > > > +++ b/utils/raspberrypi/ctt/ctt_tools.py > > > > @@ -14,7 +14,7 @@ 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 > > > > +from sklearn.neighbors import NearestCentroid as get_centroids > > > > > > > > """ > > > > This file contains some useful tools, the details of which aren't important to > > -- > Regards, > > Laurent Pinchart
diff --git a/utils/raspberrypi/ctt/ctt_tools.py b/utils/raspberrypi/ctt/ctt_tools.py index 48e0aac2..8728ff16 100644 --- a/utils/raspberrypi/ctt/ctt_tools.py +++ b/utils/raspberrypi/ctt/ctt_tools.py @@ -14,7 +14,7 @@ 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 +from sklearn.neighbors import NearestCentroid as get_centroids """ This file contains some useful tools, the details of which aren't important to
The NearestCentroid function is now in the sklearn.neighbors namespace. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- utils/raspberrypi/ctt/ctt_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)