-
Notifications
You must be signed in to change notification settings - Fork 3
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
Make gmpy2 optional #4
Comments
I like this idea! gmpy2 is a bit of a pain to setup, so this could improve usability while retaining better performance for those who want it. @NCGThompson if you or anyone else make a PR for this, I'd be happy to review it. Otherwise, I'll keep this open as enhancement to implement as time permits. |
@nickboucher What was the purpose 714cdd1? Before the commit, was the code stable and efficient? I'm thinking I might just cherrypick the change for a specific file out, so no external libraries are used for prime generation, even if they are available. |
I believe the purpose was primarily to replace Python standard lib functions with gmpy2 calls, as well as some minor logging and naming changes. It should have been stable before that, and that commit ought to be a good place to see where gmpy2 dependencies are used if you're only looking to patch a file locally. |
No. The primality check was problematic: # TODO: how many iterations?
for a in range(2, 6):
if pow(a, p - 1, p) != 1:
return False
return True There are not enough iterations, and there are certain types of composite numbers that always pass the test. |
gmpy2 relies on having gmp installed, which can be a hassle, especially is someone wants to implement this library in a stand-alone script to be run with Python3. I suggest using PyCryptodome for math functions related to cryptography (e.g.
isPrime()
). PyCryptodome will use either gmpy or gmpy2 if it is installed, but will run without it. For performance of long integer arithmetic, use the same strategy as PyCryptodome: usempz
if it is available but fallback on to the built in long type.The text was updated successfully, but these errors were encountered: