FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

✨: HasX attributes by nstarman · Pull Request #34 · data-apis/array-api-typing · GitHub

✨: HasX attributes - #34

Open
nstarman wants to merge 8 commits into
data-apis:mainfrom
nstarman:has_x_attributes
Open

✨: HasX attributes#34
nstarman wants to merge 8 commits into
data-apis:mainfrom
nstarman:has_x_attributes

Conversation

nstarman commented Jul 1, 2025
edited
Loading

Copy link
Copy Markdown
Collaborator

Requires #32. I'll rebase when that's in.

Now preceding #32.

nstarman commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator Author

@jorenham @NeilGirdhar @lucascolley this doesn't resolve the current discussion re a DTypeT typevar or a DType protocol, but does mean Arrays can now understand any typevar or protocol we want to add.

jorenham left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like the Has* protocols. Optype will cover the CanArray* ones I think. There might be some that can't be expressed because Self can't be passed as generic type argument, but I suppose we can deal with it when needed.

One nit is that ... are not needed when there's a docstring, which already counts as an expression or statement or something.

nstarman commented Jul 2, 2025

Copy link
Copy Markdown
Collaborator Author

I like the Has* protocols. Optype will cover the CanArray* ones I think. There might be some that can't be expressed because Self can't be passed as generic type argument, but I suppose we can deal with it when needed.

What do we do about docstrings?

One nit is that ... are not needed when there's a docstring, which already counts as an expression or statement or something.

Yes, my pylint is complaining. But IMO empty methods should have ... to distinguish them from ones that aren't empty. Helps in understanding inheritance with a Protocol.

jorenham commented Jul 2, 2025

Copy link
Copy Markdown
Member

What do we do about docstrings?

Hmm good question. Maybe a numpy-esque add_docstring function?

jorenham commented Jul 2, 2025

Copy link
Copy Markdown
Member

Yes, my pylint is complaining. But IMO empty methods should have ... to distinguish them from ones that aren't empty. Helps in understanding inheritance with a Protocol.

Yea I guess there's something to be said for that. It just looks a bit weird to me to see a ... occupy a line on its own (but that might have something to do with me spending most of my breathing time looking at stubs).

nstarman force-pushed the has_x_attributes branch 2 times, most recently from 8096aff to 3793c1f Compare July 21, 2025 15:56
nstarman force-pushed the has_x_attributes branch 5 times, most recently from cb3dbb3 to dec1842 Compare July 23, 2025 21:05
nstarman marked this pull request as ready for review July 23, 2025 21:10
nstarman requested a review from jorenham July 23, 2025 21:10
Comment thread src/array_api_typing/_array.py Outdated

Copy link
Copy Markdown
Collaborator Author

@jorenham I added tests and made the protocols public.

Copy link
Copy Markdown
Collaborator Author

@jorenham This should cover all the array attributes.

nstarman commented Aug 1, 2025

Copy link
Copy Markdown
Collaborator Author

@jorenham it might be easier to merge this before doing numpy type compat stuff from #32.

nstarman added this to the v2021-12-0.0 milestone Aug 1, 2025
nstarman added ✨ feature Introduce new features. ✅ tests Add, update, or pass tests. labels Aug 1, 2025
Comment thread src/array_api_typing/_array.py Outdated
Comment thread tests/integration/test_numpy2p0.pyi Outdated
Comment thread tests/integration/test_numpy2p0.pyi Outdated
Comment thread src/array_api_typing/_array.py Outdated
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
nstarman requested a review from jorenham August 20, 2025 01:26

Copy link
Copy Markdown
Contributor

Sounds great to me!!

Copy link
Copy Markdown
Member

@lucascolley WDYT?

Would you like to be a maintainer, Neil?

Good idea! Lucas is also very welcome as maintainer as far as I'm concerned.

Copy link
Copy Markdown
Member

alright, @nstarman let me know when this is ready for review.

Would you like to be a maintainer, Neil?

Good idea! Lucas is also very welcome as maintainer as far as I'm concerned.

@kgryte please could you grant me the maintainer role on this repo?

kgryte commented May 13, 2026

Copy link
Copy Markdown

@lucascolley Done.

lucascolley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I read through the whole diff, just two comments for you @nstarman, then I think we can merge it.


I also read through the discussion in the open comments. To elaborate on #34 (comment), I think we are fine to proceed in this manner, as I still think we are within our rights to change the API down the line, e.g. to provide a namespace with a restricted set of protocols whereby there is only one obvious way to achieve real use-cases.

I wonder whether, however, we should put the current developments under a sub-namespace of the package before the first release? Something like array_api_typing.experimental or array_api_typing.v0 or array_api_typing.comprehensive. That seems sufficiently non-committal to keep all stakeholders happy?

Comment thread src/array_api_typing/_array.py Outdated

Copy link
Copy Markdown
Contributor

Why do we want to export:

    "HasDType",
    "HasDevice",
    "HasMatrixTranspose",
    "HasNDim",
    "HasShape",
    "HasSize",
    "HasTranspose",

Are users going to use these in their own annotations?

Copy link
Copy Markdown
Member

There was the suggestion of #34 (comment), but I suppose I'm inclined to the view that we should in the first instance focus on useful annotations of 'array in -> array out' functions, before we branch out to cover other cases.

Copy link
Copy Markdown
Member

given the lack of movement here I am going to start merging things and see where it takes us. Don't take this as me asserting I know the best thing for the project — would be good to get all stakeholders back on the same page before making a proper release :)

Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
Signed-off-by: Lucas Colley <lucas.colley8@gmail.com>
lucascolley enabled auto-merge (squash) June 26, 2026 19:54

Copy link
Copy Markdown
Member

Waiting on 1 reapproval from someone other than the last pusher. Review from lucascolley is stale because it was submitted before the most recent code changes.

@jorenham would you do the honours? I'd like to review #69

Copy link
Copy Markdown
Member

Waiting on 1 reapproval from someone other than the last pusher. Review from lucascolley is stale because it was submitted before the most recent code changes.

@jorenham would you do the honours? I'd like to review #69

I still don't think we add functionality to the API unless there's an an actual use-case in it in an array-api library. So sorry, but I'm not going to approve this.

But don't consider this a hard veto or anything; I'm fine with anyone else approving in my place.

Copy link
Copy Markdown
Member

can I convince you to become a maintainer @NeilGirdhar ?

Copy link
Copy Markdown
Collaborator Author

Hi. I'm traveling for collaborations and conferences this past month and will be back at my normal desk in a week and able to engage with PRs and Issues.

Copy link
Copy Markdown
Contributor

can I convince you to become a maintainer @NeilGirdhar ?

Okay. I'll try to find time to help move this along 😄

Copy link
Copy Markdown
Member

Hi. I'm traveling for collaborations and conferences this past month and will be back at my normal desk in a week and able to engage with PRs and Issues.

Awesome!

Okay. I'll try to find time to help move this along 😄

@kgryte please could you add Neil to the array-api-typing team?

Copy link
Copy Markdown
Collaborator Author

@jorenham does optype do Protocol speedups like beartype for faster isinstance checks? I'm reading https://jorenham.github.io/optype/reference/#isinstance-with-protocols and I don't see anything to indicate so.

Copy link
Copy Markdown
Member

@jorenham does optype do Protocol speedups like beartype for faster isinstance checks? I'm reading jorenham.github.io/optype/reference#isinstance-with-protocols and I don't see anything to indicate so.

It doesn't, because it's not needed (anymore): beartype/numerary#20

But I'd actually advise against using runtime-checkable protocols because of the many false positives they produce (i.e. they're completely broken)

Copy link
Copy Markdown
Collaborator Author

I had a chance to look at this again. IMO we get this in and work on getting the full suite of Protocols for describing the Array API.

Copy link
Copy Markdown
Member

Yes, happy to discuss #34 (comment) in a follow-up or here

nstarman commented Jun 29, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Or here is good :).
@NeilGirdhar these are the atomic components of the Array API, and enables annotating e.g.

def get_size(x: HasShape | HasArrayNamespace, /) -> tuple[int | None, ...]:
    if hasattr(x, "shape"):
        return x.shape
    else:
        xp = get_namespace(x)
        return xp.shape(x)

a function that defers to a shape attribute, otherwise trying to use the shape function as a fallback.
They should enable precise type annotations, but also faster ones, because while runtime-accessible type Protocol aren't always desirable, they are very common, and take O(n) time checking alll n attributes of the Protocol vs O(1) for true subclass checks. Precise annotations are thus faster annotations.

lucascolley commented Jun 29, 2026
edited
Loading

Copy link
Copy Markdown
Member

to reiterate my take: happy to include these if all agree, but I have a preference for starting off with exposing the minimal public API necessary for helping type 'array in -> array out' functions. 'something similar to an array in -> something else out' can come later IMO.

NeilGirdhar commented Jun 29, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Precise annotations are thus faster annotations.

Yes, thank you. So my question was who is going to call isinstance(..., HasShape)? Is HasShape supposed to be internal to this repo, or is it supposed to be exported for users to use?

nstarman commented Jun 29, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator Author

internal vs external

IMO exposed to the user since Array would just be the multiple-inheritance over these atomic Protocols.
In my code I would prefer to use HasShape over Array where I could.

to reiterate my take: happy to include these if all agree, but I have a preference for starting off with exposing the minimal public API necessary for helping type 'array in -> array out' functions. 'something similar to an array in -> something else out' can come later IMO.

I'm happy to start that way as well, but what would that be? Like Protocol.__call__ for the functions in the Array API?

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Collaborator Author

SGTM. Many of these are like `CanAbs = opt.CanAbs[...]``

Copy link
Copy Markdown
Member

to reiterate my take: happy to include these if all agree, but I have a preference for starting off with exposing the minimal public API necessary for helping type 'array in -> array out' functions. 'something similar to an array in -> something else out' can come later IMO.

I'm happy to start that way as well

Wanna drop this down to just exposing Array at the top level then for this PR? Should be ready to merge then once the lint error is fixed as well.

Copy link
Copy Markdown
Member

gentle bump @nstarman

Copy link
Copy Markdown
Member

hey @nstarman, anything I can do to unblock this?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feature Introduce new features. ✅ tests Add, update, or pass tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL