| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
What do we do about docstrings?
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. |
Sorry, something went wrong.
Hmm good question. Maybe a numpy-esque add_docstring function? |
Sorry, something went wrong.
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). |
Sorry, something went wrong.
|
@jorenham I added tests and made the protocols public. |
Sorry, something went wrong.
|
@jorenham This should cover all the array attributes. |
Sorry, something went wrong.
Sorry, something went wrong.
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
|
Sounds great to me!! |
Sorry, something went wrong.
Good idea! Lucas is also very welcome as maintainer as far as I'm concerned. |
Sorry, something went wrong.
|
alright, @nstarman let me know when this is ready for review.
@kgryte please could you grant me the maintainer role on this repo? |
Sorry, something went wrong.
There was a problem hiding this comment.
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?
Sorry, something went wrong.
|
Why do we want to export: "HasDType",
"HasDevice",
"HasMatrixTranspose",
"HasNDim",
"HasShape",
"HasSize",
"HasTranspose",
Are users going to use these in their own annotations? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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 :) |
Sorry, something went wrong.
Co-authored-by: Lucas Colley <lucas.colley8@gmail.com> Signed-off-by: Lucas Colley <lucas.colley8@gmail.com>
|
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
can I convince you to become a maintainer @NeilGirdhar ? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
Okay. I'll try to find time to help move this along 😄 |
Sorry, something went wrong.
Awesome!
@kgryte please could you add Neil to the array-api-typing team? |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
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) |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
Yes, happy to discuss #34 (comment) in a follow-up or here |
Sorry, something went wrong.
|
Or here is good :). 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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
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? |
Sorry, something went wrong.
IMO exposed to the user since Array would just be the multiple-inheritance over these atomic Protocols.
I'm happy to start that way as well, but what would that be? Like Protocol.__call__ for the functions in the Array API? |
Sorry, something went wrong.
|
basically making https://github.com/data-apis/array-api-extra/blob/main/src/array_api_extra/_lib/_utils/_typing.pyi more accurate/complete |
Sorry, something went wrong.
|
SGTM. Many of these are like `CanAbs = opt.CanAbs[...]`` |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
hey @nstarman, anything I can do to unblock this? |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Requires #32. I'll rebase when that's in.
Now preceding #32.