ENH: Add namedtuple return types to linalg functions that return tuples #22786

asmeurer · 2022-12-13T01:27:51Z

That is, eig(), eigh(), qr(), slogdet(), and svd(). For those functions that return non-tuples with certain keyword arguments, the return type is unchanged. This change should be completely backwards compatible.

The namedtuple attribute names come from the array API specification (see, e.g.,
https://data-apis.org/array-api/latest/extensions/generated/signatures.linalg.eigh.html), with the exception of eig() which is just the same as eigh(). The name of the namedtuple object itself is not part of the specification or the public API.

I have not used a namedtuple for the tuple output for qr(mode='raw'), which returns (h, tau).

This updates the docstrings to use the updated namedtuple return names, and also the examples to use those names more consistently. This also updates the tests to check each function for the namedtuple attributes at least once.

That is, eig(), eigh(), qr(), slogdet(), and svd(). For those functions that return non-tuples with certain keyword arguments, the return type is unchanged. This change should be completely backwards compatible. The namedtuple attribute names come from the array API specification (see, e.g., https://data-apis.org/array-api/latest/extensions/generated/signatures.linalg.eigh.html), with the exception of eig() which is just the same as eigh(). The name of the namedtuple object itself is not part of the specification or the public API. I have not used a namedtuple for the tuple output for qr(mode='raw'), which returns (h, tau). This updates the docstrings to use the updated namedtuple return names, and also the examples to use those names more consistently. This also updates the tests to check each function for the namedtuple attributes at least once.

charris · 2022-12-13T19:42:24Z

numpy/linalg/linalg.py

@@ -17,6 +17,7 @@
 import functools
 import operator
 import warnings
+from typing import NamedTuple


Curious about this, it isn't the same as collections.namedtuple. Is typing.NamedTuple the compatible version?

charris · 2022-12-13T19:43:18Z

numpy/linalg/linalg.py

+    eigenvectors: NDArray
+
+class QRResult(NamedTuple):
+    Q: NDArray


Bit curious about the choice of upper case here and below. Is there a standard for these names?

This NDArray type name is already present in NumPy. Unless you mean the name of the namedtuple.

Public names of type annotations we have, like np.typing.NDArray, tend to be upper-case. I think to not confuse them with lower-case objects

I was referring to "Q".

Unless you mean the name of the namedtuple.

That was what I was referring to. Looks like NamedTuple is what we want, but am curious if the projects that we want to be compatible with also use it.

The uppercase field names are part of the array API spec and were discussed here data-apis/array-api#295. I suppose NumPy could, if it wanted, provide the same fields under multiple names if you really don't like the ones the array API chose, like

class QRResult(NamedTuple): Q: NDArray R: NDArray @property def q(self): return self.Q @property def r(self): return self.R

(although personally I think you should just use these names)

The name of the named tuple is completely arbitrary. We can change it to whatever.

As far as I can tell NamedTuple is basically equivalent to the classic namedtuple. I think the biggest difference is that NamedTuple supports type annotations, which aren't used in this file yet but presumably they will be at some point. I guess it also makes it easier to add other methods or attributes if you ever wanted to do that.

I don't think it really makes a difference whether you use NamedTuple or namedtuple. I think it could even be something else like a dataclass, so long as it also has the correct field names and also defines enough methods to act like a tuple.

As far as I can tell NamedTuple is basically equivalent to the classic namedtuple

I think NamedTuple is better, for instance, it allows accessing the fields like so: a.Q, but that wouldn't work for namedtuple. The declaration of the fields also differs, for instance the declarations here error if namedtuple is substituted. The upshot is that I think one or the other should be chosen for the standard for portable code.

namedtuple works like that too:

>>> namedtuple('test', ['a', 'b'])(1, 2).a 1

I think the only real difference is that NamedTuple lets you specify type annotations. And the class definition is arguably easier to read. As far as the user is concerned though they are identical.

charris · 2022-12-13T21:07:17Z

Could you fix the lint errors? They don't look very difficult.

seberg · 2022-12-20T12:23:20Z

I would lean towards also using a named tuple for (h, tau), it seems strange to not be consistent for a single function. But not a strong opinion.

The changes need to be copied into the typing stubs as well still. Otherwise, I expect the only thing would indeed be small style fixups to make the linter happier.

charris · 2023-02-23T23:05:29Z

The changes need to be copied into the typing stubs as well still

@asmeurer Be good to finish this up.

charris · 2023-04-04T00:54:00Z

We should finish this before the 1.25 release.

eric-wieser · 2023-04-04T22:57:12Z

The name of the namedtuple object itself is not part of the specification or the public API.

What are users expected to do when writing a type annotation for the return value, if ret : QRResult = qr(x) is using private API?

asmeurer · 2023-04-04T23:05:07Z

So maybe the namedtuple classes should be added to the API then. Maybe @BvB93 can give some insight? I'm not really knowledgeable enough about typing stuff to know the right answer here.

rgommers · 2023-04-05T08:08:56Z

I think there's still issues with NamedTuple typing in Python (e.g., this is still true probably: https://stackoverflow.com/questions/60430110/python-typing-support-for-namedtuple). I suspect that the linalg.pyi stubs for these functions need a tweak here, and that tuple unpacking will still work unchanged with mypy. Actually typing the use of qr(x).Q seems tricky.

So maybe the namedtuple classes should be added to the API then.

Not to the np.linalg API, because they're not useful for anything but static typing, and that's not enough of a reason to add them to the public (runtime) API I'd say. We do have numpy.typing for type annotations that users may need, so they could be added there perhaps? That right now has only four entries, so this would grow it quite a bit - but that seems fine. Perhaps would need to be np.typing.linalg.xxx?

eric-wieser · 2023-04-05T08:25:43Z

I suppose it would also be reasonable to think about the static types later; at worst users of heavy static typing will have to continue to use the tuple api rather than .Q and .R, which is no different to what they do today.

BvB93 · 2023-04-05T12:26:46Z

I think there's still issues with NamedTuple typing in Python (e.g., this is still true probably: https://stackoverflow.com/questions/60430110/python-typing-support-for-namedtuple). I suspect that the linalg.pyi stubs for these functions need a tweak here, and that tuple unpacking will still work unchanged with mypy. Actually typing the use of qr(x).Q seems tricky.

Runtime support for generic (i.e. parameterizable) named tuples was (re?)-introduced last year again (xref python/cpython#92027), so the issue mentioned in stackoverflow here should be irrelevant now. That, and the named tuples introduced in this PR are currently not generic anyway.

So maybe the namedtuple classes should be added to the API then. Maybe @BvB93 can give some insight? I'm not really knowledgeable enough about typing stuff to know the right answer here.

Named tuples do use a nominal sub-typing pattern just like normal classes (and contrary to protocols), so would have to expose these named tuples somewhere lest, as pointed out by Eric, the users only option would be to resort to the use of private numpy API.

That right now has only four entries, so this would grow it quite a bit - but that seems fine. Perhaps would need to be np.typing.linalg.xxx?

Either that, or if the returning of named tuples is going to be a common thing then something akin to np.typing.named_tuple maybe?

numpy/linalg/linalg.py

rgommers · 2023-04-06T11:15:29Z

Either that, or if the returning of named tuples is going to be a common thing then something akin to np.typing.named_tuple maybe?

We don't have that many functions that return multiple arrays which can be named sensibly, so I suspect this isn't necessary.

seberg · 2023-05-08T13:15:37Z

Branching for 1.25 will be approaching, might be nice to push this over the finish line.

Co-authored-by: Bas van Beek <43369155+BvB93@users.noreply.github.com>

charris · 2023-05-17T20:31:20Z

The circleci failure seems to be because the main version of .circleci/config.yml is not being used. Will ignore.

charris · 2023-05-17T21:02:43Z

Thanks @asmeurer and @BvB93 .

seberg · 2023-06-21T05:50:52Z

Just linking because I didn't really expect this pattern of someone noticing: HIPS/autograd#597 (comment)

They have a dictionary for the result types, which probably only contains tuple.

rgommers · 2023-06-21T06:12:43Z

🤔 I think I'd consider that an issue in their code, changing tuple to namedtuple seems like a thing that is okay to do, just like changing any other return type to a more specific subclass.

asmeurer added 3 commits December 12, 2022 18:23

Add a changelog entry for numpy#22786

6618fbf

Fix a doctest

271876e

charris reviewed Dec 13, 2022

View reviewed changes

seberg added this to the 1.25.0 release milestone Apr 4, 2023

BvB93 reviewed Apr 5, 2023

View reviewed changes

numpy/linalg/linalg.py Outdated Show resolved Hide resolved

numpy/linalg/linalg.py Show resolved Hide resolved

charris and others added 2 commits May 17, 2023 12:22

MAINT: Update numpy/linalg/linalg.py

610f85c

Co-authored-by: Bas van Beek <43369155+BvB93@users.noreply.github.com>

TYP: Update type annotations for the new linalg named tuples

a4c2496

charris merged commit 126b46c into numpy:main May 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add namedtuple return types to linalg functions that return tuples #22786

ENH: Add namedtuple return types to linalg functions that return tuples #22786

asmeurer commented Dec 13, 2022

charris Dec 13, 2022

charris Dec 13, 2022

asmeurer Dec 15, 2022

rgommers Dec 15, 2022

charris Dec 15, 2022

charris Dec 15, 2022

asmeurer Dec 15, 2022 •

edited

Loading

charris Dec 15, 2022

asmeurer Apr 4, 2023

charris commented Dec 13, 2022

seberg commented Dec 20, 2022

charris commented Feb 23, 2023

charris commented Apr 4, 2023

eric-wieser commented Apr 4, 2023 •

edited

Loading

asmeurer commented Apr 4, 2023

rgommers commented Apr 5, 2023

eric-wieser commented Apr 5, 2023

BvB93 commented Apr 5, 2023

rgommers commented Apr 6, 2023

seberg commented May 8, 2023

charris commented May 17, 2023

charris commented May 17, 2023

seberg commented Jun 21, 2023

rgommers commented Jun 21, 2023

ENH: Add namedtuple return types to linalg functions that return tuples #22786

ENH: Add namedtuple return types to linalg functions that return tuples #22786

Conversation

asmeurer commented Dec 13, 2022

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

asmeurer Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Dec 13, 2022

seberg commented Dec 20, 2022

charris commented Feb 23, 2023

charris commented Apr 4, 2023

eric-wieser commented Apr 4, 2023 • edited Loading

asmeurer commented Apr 4, 2023

rgommers commented Apr 5, 2023

eric-wieser commented Apr 5, 2023

BvB93 commented Apr 5, 2023

rgommers commented Apr 6, 2023

seberg commented May 8, 2023

charris commented May 17, 2023

charris commented May 17, 2023

seberg commented Jun 21, 2023

rgommers commented Jun 21, 2023

asmeurer Dec 15, 2022 •

edited

Loading

eric-wieser commented Apr 4, 2023 •

edited

Loading