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-leave@python.org
https://mail.python.org/mailman3/lists/numpy-discussion.python.org/
Member address: ralf.gommers@googlemail.com