Skip to content

MAINT: Make numpy.linalg.linalg private#24946

Merged
ngoldbaum merged 1 commit intonumpy:mainfrom
mtsokol:linalg-linalg-private
Oct 19, 2023
Merged

MAINT: Make numpy.linalg.linalg private#24946
ngoldbaum merged 1 commit intonumpy:mainfrom
mtsokol:linalg-linalg-private

Conversation

@mtsokol
Copy link
Member

@mtsokol mtsokol commented Oct 18, 2023

Hi @rgommers @ngoldbaum,

I started working on post-core-rename items to finalize NEP 52 (one thing is making sure with a test that each function is available from one place only).

In this PR I make numpy.linalg.linalg private - I think that all linalg functionality should be accessed from numpy.linalg as its __init__.py does from .linalg import *.

The only members of the submodule in question that aren't exported, and don't look private are:

from numpy.linalg._linalg import (
    QRResult, EigResult, EighResult, SVDResult, SlogdetResult
)

But these are only return named tuples, rarely used, according to github search.

@mtsokol mtsokol self-assigned this Oct 18, 2023
@mtsokol mtsokol force-pushed the linalg-linalg-private branch 2 times, most recently from 8539e2a to 27eda19 Compare October 18, 2023 11:06
@ngoldbaum
Copy link
Member

Hmm, maybe these named tuples should be public outside the typing? I have no idea if that's useful. In any case it's outside the scope of this PR. I agree, don't see any uses of any these namedtuple types outside of numpy itself, so I'm not worried about not having them publicly available.

It seems there's some usage of these namedtuples in the array API compat layer - they're copy/pasted from numpy and it looks like cupy also has versions of these tuples. It's not in the array API standard itself, though. @rgommers do you think it's worth publicly exposing these types and documenting them in the linalg docs?

Other than that this looks good to me.

@ngoldbaum
Copy link
Member

Let's bring this in, the question about making the NamedTuple types public is a separate one. Thanks @mtsokol!

@ngoldbaum ngoldbaum merged commit c0f488f into numpy:main Oct 19, 2023
@mtsokol mtsokol deleted the linalg-linalg-private branch October 19, 2023 16:18
@rgommers
Copy link
Member

Thanks for this PR!

@rgommers do you think it's worth publicly exposing these types and documenting them in the linalg docs?

No, I don't think that that is useful - I'd consider it namespace pollution without much of an upside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants