-
-
Notifications
You must be signed in to change notification settings - Fork 11.9k
API: Make intp ssize_t and introduce characters nN
#24888
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
Conversation
|
How much work is it to have both |
|
We can add a type, the question is what the default should be? I mean, I can also just add I think that:
So my thought here was that actual use is much much closer to (+In that case) We should just introduce |
fd5863b to
0d04124
Compare
|
So, the work-around for users of One caveat: I am not sure how one would add the |
01a6bb6 to
a4f96c1
Compare
f948569 to
358e31a
Compare
|
Depending on how much we worry about making pointer sized integers only available through the Fixed up the new/modified table to make a not of |
8bb75f1 to
ec3df00
Compare
|
Bleh. If I understand this correctly the question is what do we want I think this is the least bad step forward. The "full" tests failed (due to the flaky tests skipped in #25068) , if they pass I will take another look and merge it. |
These don't really make sense, even less as public API.
This also removes `NPY_INTPLTR` to avoid confusion as it would have to be `p` on old NumPy but should be `n` on new NumPy versions. I suspect all users would use `p` explicitly anyway. Since `p` is pretty clearly defined as pointer, retain that meaning.
This is probably unnecessary but for good sport seems to make sense. OpenVMS can have intptr_t at size 8 while size_t and pointers are 4 sized (because you can change the pointer size and the intptr supports either size). In general it probably doesn't care here, in principle, size_t could also be smaller than a pointer though.
(Might have gotten confused in the merge conflict?)
ce275e3 to
82d0941
Compare
|
Hum, after rebasing macos test failed with: restarted, but wondering what is up. |
|
Time to give this a shot? |
|
Thanks @seberg |
The need for this define was removed with numpygh-24888. A code search shows no external usages, and the few times this shows up on the issue tracker it's always defined to `1`.
The need for this define was removed with numpygh-24888. A code search shows no external usages, and the few times this shows up on the issue tracker it's always defined to `1`. [skip cirrus] [skip azp] [skip circle]
The need for this define was removed with numpygh-24888. A code search shows no external usages, and the few times this shows up on the issue tracker it's always defined to `1`. [skip cirrus] [skip azp] [skip circle]
The need for this define was removed with numpygh-24888. A code search shows no external usages, and the few times this shows up on the issue tracker it's always defined to `1`. [skip cirrus] [skip azp] [skip circle]
This also removes
NPY_INTPLTRto avoid confusion as it would have to bepon old NumPy but should benon new NumPy versions. I suspect all users would usepexplicitly anyway.Since
pis pretty clearly defined as pointer, retain that meaning.I proposed a few times to change this, and it is no relatively easy. The reason is that I think we have a lot of code that uses the
AsSize_Tconversion functions and generally useintpas a size, and not to store pointers (we practically never store pointers after all).There are three caveats:
pPmeanintptr_tanduintptr_tclearly, so ensure that those characters will keep mapping to that. The correct character for(s)size_tseemednN(as used by thestructmodule). (This bloats some tests a bit due toallTypesaddition)NPY_INTPLTRshould benbut would need to bepto be backwards compatible. So I just deleted it. (In principle we could define it aspunless you compile for NumPy >=2 or unlessnandpare not equivalent. But it seemed unlikely enough to be used in practice.)size_tanduintptr_tdo not have the same size, a module compiled against NumPy 2.0 cannot reliably run on 1.x, so raise an explicit error there.