MAINT unpack 0-dim NumPy array instead of implicit conversion #26345

glemaitre · 2023-05-07T12:55:31Z

Solve the deprecation warning observed in #26154

Unpack 0-dim array explicitly instead of making an implicit conversion that will raise an error in the future NumPy (1.25).

adrinjalali

in at least some of these cases, wouldn't it be better to have them as scalars rather than arrays in the first place? I'm not super comfortable with some of these hard coded indexings. Feels like we might either be missing values, or that they should have been a scalar in the first place.

glemaitre · 2023-05-15T15:08:53Z

Indeed, this is a good point. I think that for the tests, this is fine. I will look more closely at the LogisticRegressionCV.

…py_de

glemaitre · 2023-05-15T15:35:34Z

sklearn/linear_model/_logistic.py

@@ -506,6 +506,9 @@ def _logistic_regression_path(
                w0 = np.concatenate([coef_.ravel(), intercept_])
            else:
                w0 = coef_.ravel()
+            # n_iter_i is an array for each class. However, `target` is always encoded
+            # in {-1, 1}, so we only take the first element of n_iter_i.
+            n_iter_i = n_iter_i[0]


So here this is really specific to liblinear. We should preserve the array because it will be useful in multiclass case in other part of the code.

glemaitre · 2023-05-15T16:11:56Z

sklearn/tree/_export.py

            else:
                alpha = (sorted_values[0] - sorted_values[1]) / (1 - sorted_values[1])
        else:
            # Regression tree or multi-output
+            if isinstance(value, Iterable):
+                # regression tree


I added the reason why we cannot unpack the value earlier.

glemaitre

I modified the places and check it makes sense to unpack value.
I added a comment why the unpacking is happening such that the future us will not find this unpacking weird.

glemaitre · 2023-05-15T16:38:07Z

This is a codecov failure.

adrinjalali

Otherwise LGTM.

adrinjalali · 2023-05-16T09:11:02Z

sklearn/tree/_export.py

@@ -247,17 +248,21 @@ def get_color(self, value):
            color = list(self.colors["rgb"][np.argmax(value)])
            sorted_values = sorted(value, reverse=True)
            if len(sorted_values) == 1:
-                alpha = 0
+                alpha = 0.0
            else:
                alpha = (sorted_values[0] - sorted_values[1]) / (1 - sorted_values[1])
        else:
            # Regression tree or multi-output


so here it's never multi-output? if it is, we shouldn't take the first value, should we?

Apparently, the multi-output case is when value is not an Iterable.

O_o then what is it? that seems very shaky, is there another way we could check for multioutput? this right now seems very confusing.

In this case, I prefer to unpack from the call-site:

scikit-learn/sklearn/tree/_export.py

Lines 277 to 281 in a2b8571

if tree.n_outputs == 1:

node_val = tree.value[node_id][0, :] / tree.weighted_n_node_samples[node_id]

if tree.n_classes[0] == 1:

# Regression

node_val = tree.value[node_id][0, :]

and leave get_color alone.

So I let get_color alone :)
I added a comment to differentiate the regression case from the classification from single class

thomasjpfan

Usually I use ndarray.item to unpacking 0-dim ndarrays into Python scalars. Using item is a little safer, because it will raise an error if the underlying array has more than one element.

thomasjpfan · 2023-05-16T13:38:40Z

sklearn/tree/_export.py

@@ -247,17 +248,21 @@ def get_color(self, value):
            color = list(self.colors["rgb"][np.argmax(value)])
            sorted_values = sorted(value, reverse=True)
            if len(sorted_values) == 1:
-                alpha = 0
+                alpha = 0.0
            else:
                alpha = (sorted_values[0] - sorted_values[1]) / (1 - sorted_values[1])
        else:
            # Regression tree or multi-output


In this case, I prefer to unpack from the call-site:

scikit-learn/sklearn/tree/_export.py

Lines 277 to 281 in a2b8571

if tree.n_outputs == 1:

node_val = tree.value[node_id][0, :] / tree.weighted_n_node_samples[node_id]

if tree.n_classes[0] == 1:

# Regression

node_val = tree.value[node_id][0, :]

and leave get_color alone.

jeremiedbb

nitpicks

sklearn/linear_model/tests/test_sag.py

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

glemaitre · 2023-06-09T09:32:12Z

I used item instead of indexing as suggested by @thomasjpfan

adrinjalali

This looks much nicer now. Thanks @glemaitre

jeremiedbb

LGTM. I triggered a scipy dev build to check if we're not missing any of these

glemaitre · 2023-06-09T13:07:08Z

The failure seems to be some other failures :)

jeremiedbb · 2023-06-09T13:07:36Z

I checked that all remaining failures are unrelated to 0 dim arrays. Let's merge

…-learn#26345) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

glemaitre added 5 commits May 7, 2023 14:11

MAINT index 0-dim array in _random_choice_csc

a48e127

MAINT unpack 0-dim array in get_color

5b2464d

MAINT unpack coef and intercept from 0-dim array

889b127

MAINT unpack 0-dim scalar in test

67faaf5

[scipy-dev] trigger ci

7dd783e

github-actions bot added module:linear_model module:tree module:utils labels May 7, 2023

glemaitre added the No Changelog Needed label May 7, 2023

glemaitre added 3 commits May 7, 2023 19:08

TST unpack scalar from 0-dim array

6f16830

iter

e356712

[scipy-dev] trigger ci

8a90526

adrinjalali reviewed May 8, 2023

View reviewed changes

glemaitre added 2 commits May 15, 2023 17:09

Merge remote-tracking branch 'origin/main' into deprecation_numpy_sci…

92ef32d

…py_de

[scipy-dev] make it explicit why we get the single element

1b4818a

glemaitre commented May 15, 2023

View reviewed changes

glemaitre added 3 commits May 15, 2023 17:40

be more explicit

12b989e

unpack value from regression tree

266bfea

[scipy-dev] iter

f7d2111

glemaitre commented May 15, 2023

View reviewed changes

adrinjalali reviewed May 16, 2023

View reviewed changes

Merge branch 'main' into deprecation_numpy_scipy_de

742cada

thomasjpfan reviewed May 16, 2023

View reviewed changes

jeremiedbb reviewed Jun 7, 2023

View reviewed changes

sklearn/linear_model/tests/test_sag.py Outdated Show resolved Hide resolved

sklearn/linear_model/tests/test_sag.py Outdated Show resolved Hide resolved

glemaitre and others added 3 commits June 9, 2023 10:33

Merge branch 'main' into deprecation_numpy_scipy_de

ad3d2a1

Apply suggestions from code review

39ec6cb

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

iter

8514207

use item instead of indexing

b021f1d

adrinjalali approved these changes Jun 9, 2023

View reviewed changes

[azure parallel] [scipy-dev]

a1ade88

jeremiedbb approved these changes Jun 9, 2023

View reviewed changes

jeremiedbb merged commit 9eea5b7 into scikit-learn:main Jun 9, 2023

manudarmi pushed a commit to primait/scikit-learn that referenced this pull request Jun 12, 2023

MAINT unpack 0-dim NumPy array instead of implicit conversion (scikit…

53df68c

…-learn#26345) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

Micky774 mentioned this pull request Aug 18, 2023

FIX NumPy DeprecationWarning in a rare branch of _lars_path_solver #27079

Merged

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023

MAINT unpack 0-dim NumPy array instead of implicit conversion (scikit…

dc5bea4

…-learn#26345) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

lucyleeow mentioned this pull request Apr 10, 2025

Array API support for pairwise kernels #29822

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT unpack 0-dim NumPy array instead of implicit conversion #26345

MAINT unpack 0-dim NumPy array instead of implicit conversion #26345

	if tree.n_outputs == 1:
	node_val = tree.value[node_id][0, :] / tree.weighted_n_node_samples[node_id]
	if tree.n_classes[0] == 1:
	# Regression
	node_val = tree.value[node_id][0, :]

MAINT unpack 0-dim NumPy array instead of implicit conversion #26345

MAINT unpack 0-dim NumPy array instead of implicit conversion #26345

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment