Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: modify namespace based on backend #61

Merged
merged 5 commits into from
Dec 30, 2024
Merged

ENH: modify namespace based on backend #61

merged 5 commits into from
Dec 30, 2024

Conversation

mdhaber
Copy link
Owner

@mdhaber mdhaber commented Dec 29, 2024

Closes gh-59

@lucascolley I think this is what you had in mind for gh-59

import numpy as np
import array_api_strict as xp
import marray
mnp = marray.get_namespace(np)
mxp = marray.get_namespace(xp)
xp1 = mnp.arange(2).__array_namespace__()
xp1.__name__  # 'mxp(numpy)'
xp1.arange(2).data.__array_namespace__().__name__  # 'numpy'
xp2 = mxp.arange(3).__array_namespace__()
xp2.__name__  # 'mxp(array_api_strict)'
xp2.arange(3).data.__array_namespace__().__name__  # 'array_api_strict'

A problem is that this no longer works:

from mnp import asarray
# ModuleNotFoundError: No module named 'mnp'
from mxp(numpy) import asarray
# SyntaxError: invalid syntax

I don't think we can tell the name that the user assigns the namespace to, and I don't think we can have parentheses in the name we want to be able to import from. I can think of a few ways to address these issues, but I'll let you suggest first.

One (orthogonal) thought is that maybe the name should involve marray instead of mxp. Another is that maybe get_namespace should be marray.

@lucascolley
Copy link
Collaborator

lucascolley commented Dec 29, 2024

One (orthogonal) thought is that maybe the name should involve marray instead of mxp

+1

Another is that maybe get_namespace should be marray.

I'm generally not a fan of from foo import foo, but I agree that something like that seems better than get_namespace. Having just looked at hgrecco/pint#2101, I like the sound of pint_namespace(xp) to produce a pint(xp) namespace, but marray has a couple more syllables 😅

from mnp import asarray

what is the motivation for that working? I don't think I understand the use-case.

@mdhaber
Copy link
Owner Author

mdhaber commented Dec 30, 2024

what is the motivation for that working?

Principle? Ideally, I think it would work "just like a module" rather than the alternative of "sort of like a module"

@lucascolley
Copy link
Collaborator

Ideally, I think it would work "just like a module" rather than the alternative of "sort of like a module"

Does xp = array_namespace(x) do that? I'd say that is the behaviour to match.

@mdhaber
Copy link
Owner Author

mdhaber commented Dec 30, 2024

No. You mean array_api_compat?

from array_api_compat import array_namespace
import numpy as np
x = np.asarray(1)
xp = array_namespace(x)
from xp import asarray

Why is that the behavior to match?

I suppose it depends on whether this is mostly for end users or a tool for developers of libraries that work with any backend.

For library developers, I agree that array_namespace accepting an existing array and returning a namespace is a good way to go. I suppose them marray_namespace could behave similarly.

mxp = marray_namespace(x)

But that's not what I've had in mind.

I've thought of

mxp = get_namespace(np)

as something like an import statement. If it could be written as

import marray(numpy)

I would prefer that.

@lucascolley
Copy link
Collaborator

Right, I can see how that would be more intuitive for users. I would say we could punt on it until it is requested though.

@mdhaber
Copy link
Owner Author

mdhaber commented Dec 30, 2024

How about this:

from marray import numpy as mnp
mnp
# <module 'marray.numpy'>
mnp.asarray([1, 2, 3])
# MArray(array([1, 2, 3]), array([False, False, False]))

import numpy  # there's no conflict
numpy  # <module 'numpy' from 'C:\\Users\\matth\\Desktop\\marray\\.pixi\\envs\\dev\\Lib\\site-packages\\numpy\\__init__.py'>

from marray import array_api_strict as mxp
# <module 'marray.array_api_strict'>
mxp.asarray([1, 2, 3])
# MArray(
#     Array([1, 2, 3], dtype=array_api_strict.int64),
#     Array([False, False, False], dtype=array_api_strict.bool)
# )

That brings the number of lines to get up and running from four down to two.
I think I'd be happy for that to be the primary way we show in the tutorial. We wouldn't need get_namespace that accepts and returns a namespace to be public. Instead, if array libraries would want a marray_namespace function to accept an array and return the corresponding masked namespace, we could do that.

Incidentally...

from marray import numpy
from marray.numpy import asarray  # works

@lucascolley
Copy link
Collaborator

that's clever! sure thing

@mdhaber mdhaber marked this pull request as ready for review December 30, 2024 18:13
@mdhaber
Copy link
Owner Author

mdhaber commented Dec 30, 2024

If CI is green, I'm happy with this. Follow-up work:

  • I'll add an marray_namespace function that behaves like array_namespace
  • @lucascolley if you could regenerate the array_api_tests patch properly and use from marray import array_api_strict as xp instead of the private _get_namespace, that would be great. I've never worked with patch files, and making too many changes corrupted it. But I think what I have is fine for this PR.

Copy link
Collaborator

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I think I still prefer foo(bar(...)) in the context of wrapping to foo.bar...., but I think it's fine given that it matches the import structure

@mdhaber mdhaber requested a review from lucascolley December 30, 2024 18:17
@lucascolley
Copy link
Collaborator

@lucascolley if you could regenerate the array_api_tests patch properly and use from marray import array_api_strict as xp instead of the private _get_namespace, that would be great. I've never worked with patch files, and making too many changes corrupted it. But I think what I have is fine for this PR.

the patch file nonsense will be banished once data-apis/array-api-tests#334 is reviewed :)

@mdhaber mdhaber merged commit f105f19 into main Dec 30, 2024
11 checks passed
@lucascolley
Copy link
Collaborator

actually, you might not even need to wait for that PR if marray.array_api_strict works

@mdhaber
Copy link
Owner Author

mdhaber commented Dec 30, 2024

You can't just import marray.array_api_strict, I don't think, but marray.array_api_strict works as a namespace after from marray import array_api_strict.

@lucascolley
Copy link
Collaborator

You can't just import marray.array_api_strict, I don't think, but marray.array_api_strict works as a namespace after from marray import array_api_strict.

Ah, true, but fortunately, if you give array-api-tests the former in an env-var, it executes the latter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

__array_namespace__() magic methods
2 participants