-
Notifications
You must be signed in to change notification settings - Fork 12
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
pcg32bounded argument check #4
Comments
I implemented the change, and it is faster... Before...
After...
My code differs slightly from yours, but should be equivalent. Thanks. |
You welcome. Some bad output and error messages with your patch.
My PyLong_AsLong patch may be better. It handled all the error message as expected. |
Can you verify your exact patch on modern Pythons? Python 2.7 and 3.6? I do not get at all what you are getting with your patch. And I don't think it makes sense to install an old Python. |
My patch verified OK with Python latest 3.6.5 (Windows x86 executive installer)
Perhaps you are using 64-bits system ? What unexpected output are you getting ? |
I merged your code (see GitHub). I used to get an error when inputting a negative value, now I just get nothing. It does not sound like progress to me. See:
|
I would much rather get a SystemError than nothing when a negative value is used. |
You mean like this ? (I picked ValueError, for SystemError, use PyExc_SystemError)
This is the patch:
|
You are correct. This is python gmpy2 behavior for non-exist values.
|
To match pcg32bounded() doc-string (below), I propose to patch argument check.
Sorry, this should be a pull-request, but I don't know how to use it
Since pcg32bounded only allowed 1 argument, the code can be optimized
without overhead of parsing tuple. Just go straight for the number.
This is the full patch:
With the patch, this is benchmark on my laptop:
Error messages now as expected:
The text was updated successfully, but these errors were encountered: