Skip to content

✨ HasDType, Array #48

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 23, 2025
Merged

✨ HasDType, Array #48

merged 3 commits into from
Jul 23, 2025

Conversation

nstarman
Copy link
Collaborator

@nstarman nstarman commented Jul 22, 2025 •
edited
Loading

Relies on #47

A new PR to precede #32.

@nstarman nstarman added this to the v2021-12-0.0 milestone Jul 22, 2025
@nstarman nstarman force-pushed the has-dtype branch 2 times, most recently from 9b98cc1 to b8cc51d Compare July 22, 2025 14:43
jorenham
jorenham reviewed Jul 22, 2025
View reviewed changes
Copy link
Member

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

sorry if this is premature, but maybe these comments can help 🤷🏻

tests/integration/test_numpy2.pyi Outdated
@@ -1,11 +1,52 @@
from typing import Any
# mypy: disable-error-code="no-redef"
Copy link
Member

Choose a reason for hiding this comment

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

is this needed for the redefinition of _?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't want to name all the variables.

Copy link
Member

Choose a reason for hiding this comment

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

There are only two hard things in Computer Science: cache invalidation and naming things.

-- Phil Karlton

Copy link
Member

Choose a reason for hiding this comment

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

Or even better:

There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors.

-- Leon Bambrick

@nstarman nstarman force-pushed the has-dtype branch 3 times, most recently from f203d12 to 5ad0d5c Compare July 22, 2025 15:14
@nstarman nstarman added 🎨 structure/format Improve structure / format of the code. ✨ feature Introduce new features. 👷 CI Add or update CI build system. labels Jul 22, 2025
@nstarman nstarman force-pushed the has-dtype branch 4 times, most recently from 3434139 to 0e79721 Compare July 22, 2025 15:22
@nstarman
Copy link
Collaborator Author

numpy 2.0's array_api stuff is infinitely better than 1.0's. It's a real struggle to write tests that both pass and test what we want. 😮‍💨

@nstarman nstarman marked this pull request as ready for review July 22, 2025 15:23
@nstarman nstarman requested a review from jorenham July 22, 2025 15:25
@nstarman
Copy link
Collaborator Author

nstarman commented Jul 22, 2025 •
edited
Loading

@jorenham. Ok. this PR was much easier than #32 and hopefully will help with that one, particularly in writing more comprehensive tests. Might go one Can* binop at a time given some of the complexity with bool-dtyped arrays.
But we should probably have more array libraries being tested... and other type checkers doing the testing...

@nstarman
Copy link
Collaborator Author

Pushed to add docs to HasArrayNamespace

@nstarman
🚚 move HasArrayNamespace
255f453 
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman nstarman changed the title ✨ HasDType ✨ HasDType, Array Jul 22, 2025
nstarman added 2 commits July 22, 2025 11:45
@nstarman
✨Array class
65256af 
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman
✨HasDType
0e90681 
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman
Copy link
Collaborator Author

Forgot to have Array inherit from HasArrayNamespace. That's kind of the most important one 😆

@nstarman
Copy link
Collaborator Author

nstarman commented Jul 22, 2025 •
edited
Loading

Ok. @jorenham now it should really be ready for a review.

@jorenham
Copy link
Member

numpy 2.0's array_api stuff is infinitely better than 1.0's. It's a real struggle to write tests that both pass and test what we want. 😮‍💨

Maybe we could use array-api-compat for numpy<2.0?

@nstarman
Copy link
Collaborator Author

That's a good idea. After this PR because things are passing?

@jorenham
Copy link
Member

But we should probably have more array libraries being tested... and other type checkers doing the testing...

Yea, I'll get on that soon (after I fix optype and scipy-stubs apparently being broken on numpy<2.3 😅).

jorenham
jorenham approved these changes Jul 23, 2025
View reviewed changes
Copy link
Member

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

should be good to merge

@nstarman nstarman mentioned this pull request Jul 23, 2025
Open
@nstarman nstarman merged commit bacf0f9 into data-apis:main Jul 23, 2025
16 checks passed
@nstarman nstarman deleted the has-dtype branch July 23, 2025 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorenham jorenham jorenham approved these changes

Assignees
No one assigned
Labels
👷 CI Add or update CI build system. ✨ feature Introduce new features. 🎨 structure/format Improve structure / format of the code.
Projects
None yet
Milestone
v2021-12-0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@nstarman @jorenham