Skip to content

TASK,TYP: Review np.long related (typing) overloads and tests #25048

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

Closed
1 of 2 tasks
BvB93 opened this issue Nov 1, 2023 · 6 comments
Closed
1 of 2 tasks

TASK,TYP: Review np.long related (typing) overloads and tests #25048

BvB93 opened this issue Nov 1, 2023 · 6 comments

Comments

@BvB93
Copy link
Member

BvB93 commented Nov 1, 2023

Proposed new feature or change:

xref #25039 and #24224

Aforementioned PRs have exposed a number of cases wherein the np.long re-introduction and shift to a new default int size of 64 bit have resulted in a number of failures in the typing test suite. These cases should be further reviewed and addressed appropriately.

In particular this concerns the:

  • np.ctypeslibs reveal tests
  • np.random reveal tests
@BvB93 BvB93 added this to the 2.0.0 release milestone Nov 1, 2023
@BvB93 BvB93 changed the title TASK: Review np.long related overloads TASK,TYP: Review np.long related (typing) overloads and tests Nov 1, 2023
@seberg
Copy link
Member

seberg commented Nov 1, 2023

I think one reason why I got stuck multiple times is that it is easy to forget a definition, and that seems to translate to Any. I would guess mypy gives at least a warning for that, maybe even an error?
Could we make sure we notice undefined variables in the stubs when running tests?

EDIT: as of writing gh-24224 may have left _NBitInt and the way the aliases work in a (presumably OK), but not ideal state.

@rgommers
Copy link
Member

@BvB93 I think we want to keep the 2.0.0 milestone for this, but it's probably not blocking for the release branch creation and 2.0.0rc1, right?

@BvB93
Copy link
Member Author

BvB93 commented Feb 3, 2024

Correct, I'll see if I can make some time next week to review the issue but this is by no means a blocker.

@rgommers
Copy link
Member

The numpy.random annotations were updated in gh-26089, which is already backported to 2.0.0.

@mtsokol @BvB93 did you mean to leave this open because ctypeslib (and maybe something else) still needs reviewing?

@mtsokol
Copy link
Member

mtsokol commented Mar 27, 2024

@rgommers For the ctypeslib reveal test where we have a separate if for win32 for np.long, I don't think there's much to do about it - in CPython when intc is the same size as long, c_int is an alias to c_long:

https://github.com/python/cpython/blob/92397d5ead38dde4154e70d00f24973bcf2a925a/Lib/ctypes/__init__.py#L183

From my side there's nothing else left here.

@seberg
Copy link
Member

seberg commented Mar 27, 2024

Let's close it for now then, if there is something remaining it's a bug now.

@seberg seberg closed this as completed Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants