Fix power of Integer with modulus 0#41695
Conversation
|
Documentation preview for this PR (built with commit e6673c6; changes) is ready! 🎉 |
|
For what it's worth, Python throws a |
|
I think raising a We can also raise an if modulus == 0:
return pow(left, right) % modulusso that we might get consistent with Also there are cases when To me, that So ... we can do
What do you think is better? |
|
LGTM left this to @yyyyx4 |
sagemathgh-41695: Fix power of `Integer` with modulus `0` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> When calling `Integer.__pow__` with modulus `0`, `Mod(left, modulus)` is an Integer, instead of `IntegerMod_int`, hence no `(Mod(left, modulus) ** right).lift` method or calls the wrong `lift` method, e.g. ```Python pow(ZZ(-1), QQ((1, 2)), 0) # => x, a polynomial, since it calls I.lift pow(ZZ(3), 2, 0) # => AttributeError: 'sage.rings.integer.Integer' object has no attribute 'lift' ``` I add a guard against `modulus == 0` so that `pow(n, ext, 0)` would be the same as `pow(n, ext)`. A related doctest is added. Fixes sagemath#41692 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41695 Reported by: Majorana Oedipus Reviewer(s): Chenxin Zhong
sagemathgh-41695: Fix power of `Integer` with modulus `0` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> When calling `Integer.__pow__` with modulus `0`, `Mod(left, modulus)` is an Integer, instead of `IntegerMod_int`, hence no `(Mod(left, modulus) ** right).lift` method or calls the wrong `lift` method, e.g. ```Python pow(ZZ(-1), QQ((1, 2)), 0) # => x, a polynomial, since it calls I.lift pow(ZZ(3), 2, 0) # => AttributeError: 'sage.rings.integer.Integer' object has no attribute 'lift' ``` I add a guard against `modulus == 0` so that `pow(n, ext, 0)` would be the same as `pow(n, ext)`. A related doctest is added. Fixes sagemath#41692 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41695 Reported by: Majorana Oedipus Reviewer(s): Chenxin Zhong
sagemathgh-41695: Fix power of `Integer` with modulus `0` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> When calling `Integer.__pow__` with modulus `0`, `Mod(left, modulus)` is an Integer, instead of `IntegerMod_int`, hence no `(Mod(left, modulus) ** right).lift` method or calls the wrong `lift` method, e.g. ```Python pow(ZZ(-1), QQ((1, 2)), 0) # => x, a polynomial, since it calls I.lift pow(ZZ(3), 2, 0) # => AttributeError: 'sage.rings.integer.Integer' object has no attribute 'lift' ``` I add a guard against `modulus == 0` so that `pow(n, ext, 0)` would be the same as `pow(n, ext)`. A related doctest is added. Fixes sagemath#41692 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41695 Reported by: Majorana Oedipus Reviewer(s): Chenxin Zhong
sagemathgh-41695: Fix power of `Integer` with modulus `0` <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> When calling `Integer.__pow__` with modulus `0`, `Mod(left, modulus)` is an Integer, instead of `IntegerMod_int`, hence no `(Mod(left, modulus) ** right).lift` method or calls the wrong `lift` method, e.g. ```Python pow(ZZ(-1), QQ((1, 2)), 0) # => x, a polynomial, since it calls I.lift pow(ZZ(3), 2, 0) # => AttributeError: 'sage.rings.integer.Integer' object has no attribute 'lift' ``` I add a guard against `modulus == 0` so that `pow(n, ext, 0)` would be the same as `pow(n, ext)`. A related doctest is added. Fixes sagemath#41692 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41695 Reported by: Majorana Oedipus Reviewer(s): Chenxin Zhong
When calling
Integer.__pow__with modulus0,Mod(left, modulus)is an Integer, instead ofIntegerMod_int, hence no(Mod(left, modulus) ** right).liftmethod or calls the wrongliftmethod, e.g.I add a guard against
modulus == 0so thatpow(n, ext, 0)would be the same aspow(n, ext). A related doctest is added.Fixes #41692
📝 Checklist
⌛ Dependencies