[Numpy-discussion] Re: Add diagonal offset argument to all functions that are missing it
On Sun, Feb 9, 2025 at 6:34 PM Carlos Martin wrote: > The following functions accept a diagonal offset argument: > - https://numpy.org/doc/stable/reference/generated/numpy.diag.html > - https://numpy.org/doc/stable/reference/generated/numpy.diagflat.html > - https://numpy.org/doc/stable/reference/generated/numpy.diagonal.html > - https://numpy.org/doc/stable/reference/generated/numpy.eye.html > - https://numpy.org/doc/stable/reference/generated/numpy.trace.html > - https://numpy.org/doc/stable/reference/generated/numpy.tri.html > - https://numpy.org/doc/stable/reference/generated/numpy.tril.html > - https://numpy.org/doc/stable/reference/generated/numpy.tril_indices.html > - > https://numpy.org/doc/stable/reference/generated/numpy.tril_indices_from.html > - https://numpy.org/doc/stable/reference/generated/numpy.triu.html > - https://numpy.org/doc/stable/reference/generated/numpy.triu_indices.html > - > https://numpy.org/doc/stable/reference/generated/numpy.triu_indices_from.html > > The following functions lack such an argument: > - https://numpy.org/doc/stable/reference/generated/numpy.diag_indices.html > - > https://numpy.org/doc/stable/reference/generated/numpy.diag_indices_from.html > - > https://numpy.org/doc/stable/reference/generated/numpy.fill_diagonal.html > - https://github.com/numpy/numpy/issues/14402 > - https://github.com/numpy/numpy/issues/18000 > - https://github.com/numpy/numpy/pull/15079 > - https://numpy.org/doc/stable/reference/generated/numpy.identity.html > > Feature request: Add a diagonal offset argument to all of the functions > that are missing it. (fill_diagonal is already tracked by the > aforementioned issues, but the rest aren't, to my knowledge.) > This sounds quite reasonable to me. The `k=0` keyword is quite badly named, which is my one concern. Especially when tacking it on at the end of a signature with already 3-4 keywords, it's not a good name. How about something like `diag_offset`? Cheers, Ralf ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
[Numpy-discussion] Re: Add diagonal offset argument to all functions that are missing it
Ralf Gommers wrote: > This sounds quite reasonable to me. The `k=0` keyword is quite badly named, > which is my one concern. Especially when tacking it on at the end of a > signature with already 3-4 keywords, it's not a good name. How about > something like `diag_offset`? FWIW, we chose `offset` for `array_api_extra.create_diagonal`, instead of inheriting `k` from `np.diag`. `np.diagonal` and `np.linalg.trace` also use `offset`. Given that all of the proposed functions apart from `np.identity` already have "diag" as a substring of their name, I think just `offset` would be fine. What else could `offset` mean in the case of `np.identity`? I suppose there is an argument for actually leaving `np.identity` as is—if someone wants a square off-diagonal matrix of ones, which isn't an identity matrix, their code might be more readable with `np.diag(np.ones(...), k=1)` or just `np.eye` instead, right? Maybe it is weird for "eye" but not "identity" to have this capability, though. Cheers, Lucas ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
[Numpy-discussion] Re: Add diagonal offset argument to all functions that are missing it
On Tue, Feb 11, 2025 at 10:55 AM Lucas Colley via NumPy-Discussion < numpy-discussion@python.org> wrote: > Ralf Gommers wrote: > > This sounds quite reasonable to me. The `k=0` keyword is quite badly > named, > > which is my one concern. Especially when tacking it on at the end of a > > signature with already 3-4 keywords, it's not a good name. How about > > something like `diag_offset`? > > FWIW, we chose `offset` for `array_api_extra.create_diagonal`, instead of > inheriting `k` from `np.diag`. `np.diagonal` and `np.linalg.trace` also use > `offset`. Given that all of the proposed functions apart from `np.identity` > already have "diag" as a substring of their name, I think just `offset` > would be fine. What else could `offset` mean in the case of `np.identity`? > `offset` sounds good to me. > I suppose there is an argument for actually leaving `np.identity` as is—if > someone wants a square off-diagonal matrix of ones, which isn't an identity > matrix, their code might be more readable with `np.diag(np.ones(...), k=1)` > or just `np.eye` instead, right? Maybe it is weird for "eye" but not > "identity" to have this capability, though. > Yeah I was thinking about commenting on that - I can see an argument for not touching `identity`. But then I thought it's the same for some other matrices, like a diagonal or triangular one - with an offset, they don't really meet their mathematical description anymore. So I am can see arguments either way. Cheers, Ralf > > Cheers, > Lucas > ___ > NumPy-Discussion mailing list -- numpy-discussion@python.org > To unsubscribe send an email to numpy-discussion-le...@python.org > https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ > Member address: ralf.gomm...@googlemail.com > ___ NumPy-Discussion mailing list -- numpy-discussion@python.org To unsubscribe send an email to numpy-discussion-le...@python.org https://mail.python.org/mailman3/lists/numpy-discussion.python.org/ Member address: arch...@mail-archive.com
