Skip to content
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

Improve NotImplementedError for sqrt(Mod(a,n), all=True) #39856

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

EigenVector22
Copy link

Fixes #38802
When sqrt(Mod(a,n), all=True) is called for a non-square a with the default extend=True, the resulting NotImplementedError was very generic.

Improved the error message to suggest trying extend=False to find roots only within the base ring Zmod(n), as discussed in the issue.

Included doctest .

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

When sqrt(Mod(a,n), all=True) is called for a non-square 'a' with the default extend=True, the resulting NotImplementedError was very generic.

Improved the error message to suggest trying extend=False to find roots only within the base ring Zmod(n), as discussed in the issue.

Included a doctest .
@EigenVector22 EigenVector22 force-pushed the ticket/38802-sqrt-mod-error-msg branch from 616133e to ed69335 Compare April 3, 2025 13:58
@EigenVector22
Copy link
Author

@maxale hello, can you review this and give feedback for the change made,
Thankss,

@maxale
Copy link
Contributor

maxale commented Apr 3, 2025

@maxale hello, can you review this and give feedback for the change made, Thankss,

It looks fine to me, but I'm not a reviewer here.

@EigenVector22
Copy link
Author

@user202729 can you please review this and give feedback,
thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqrt(Mod(,),all=True) should raise more informative NotImplmentedError
2 participants