NotImplementedError when pow() called with modulus

[kylehofmann]

Many polynomial types silently ignore the modulus (third) argument of pow(). For example,

sage: R.<x> = PolynomialRing(ZZ)
sage: pow(x, 2, x)
x^2

This is true of LaurentPolynomial, LaurentPolynomial_mpair, MPolynomial_libsingular, Polynomial_integer_dense_flint, Polynomial_rational_flint, and NCPolynomial_plural. Ideally these classes would implement this argument, but at the very least they should raise NotImplementedError instead of giving an incorrect result.

[github-actions]

Documentation preview for this PR (built with commit 188f93f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[user202729]

Sounds like a good idea but there's a significant code duplication going on… is there a way to eliminate that? (for example with function decoration, or if it's too slow, at least a utility function that can be called?)

[kylehofmann]

I also generally don't like duplicating code, but in this case, most of the duplication is in the unit tests. Those need to be near what they're testing, and they have minor differences because they test different classes. Except for those, the only duplicated code is two statements long (an if and a raise). You could reduce it to two lines if you allowed wider lines (or used a shorter, more obscure error message) or one line with an auxiliary function (I don't think a decorator is a good idea here), but there is also the risk that using such a function would make the code less clear (particularly for whoever eventually implements pow with a modulus). I think this version is acceptable, but I'd be happy to listen to other opinions.

[vincentmacri]

One way to reduce the code duplication could be something like a __check_pow_parameters method that raises an error if mod is not None in whatever base class all these polynomial types inherit from, and call it in the __pow__ method for each polynomial type. But I think that what was submitted here is fine as-is.

@kylehofmann What's going on with the test failure in plural.pyx?

[kylehofmann]

I don't know why that file's tests fail, but I believe it's not an issue with my code. You can find other MRs where the test process has failed, like #38748. When I test that file myself, or even when I run all tests, I don't get any errors.

[vincentmacri]

I don't know why that file's tests fail, but I believe it's not an issue with my code. You can find other MRs where the test process has failed, like #38748. When I test that file myself, or even when I run all tests, I don't get any errors.

Yeah, I see a similar test failure in the PR you linked. I was concerned since the test failure was in a file you changed, but it seems to be a coincidence that you just happened to change a file where the CI is having issues.

[vincentmacri]

LGTM