add fmpz_is_square

[roos-j]

No description provided.

[oscarbenjamin]

We can have both methods but more useful than is_perfect_power would be a method that returns the power like SymPy's perfect_power function:

In [1]: perfect_power(81)
Out[1]: (3, 4)

In [2]: perfect_power(80)
Out[2]: False

I'm not sure what is best to return if it is not a perfect power. Returning False is perhaps a bit strange...

Alternatives:

1. Raise an exception.
2. Return clearly invalid number like (0, -1).
3. Return a tuple with a boolean like (3, 4, True) or (0, -1, False).
4. Return None.

Maybe returning False is good as anything else...

[oscarbenjamin]

Or perhaps it can just be:

>>> perfect_power(80)
(80, 1)

So perfect_power(n) always returns (r, k) such that n = r^k where k is either maximal or 1 if no maximal k exists:

>>> perfect_power(1)
(1, 1)

[roos-j]

is_perfect_power was there before this PR, but when I saw it, I also thought it should best return (r,k) since that data is provided by the C function (in that case I would lean towards None if not a perfect power), so I'd be happy to change that.
Are you also suggesting it be renamed to perfect_power? That could make sense since one might expect a method called is_something to return a boolean. On the other hand, it seems the currently prevailing convention is to mirror the C library names, so maybe it should stay is_perfect_power?

[oscarbenjamin]

Sorry I missed the fact that the function was already there. I guess it is better not to change it then.

We could add a separate .perfect_power() method though to return (r, k). The convention it seems in other places is that a perfect power should have r,k>=2 although I guess that the .perfect_power() method should be consistent with is_perfect_power().

[roos-j]

I think that would be good. The issue is though that (r,k) returned by fmpz_is_perfect_power is not guaranteed to have maximal k; the right way to proceed would be to implement that feature in flint's fmpz_is_perfect_power and then use it in python-flint to implement a method .perfect_power() as you are suggesting.

[oscarbenjamin]

Ah, okay. I suppose we could just call fmpz_is_perfect_power in a loop until we've reduced down to a number that is not a perfect power.

Maybe returning a result with non-maximal k is useful. I would be more inclined to reduce as far as possible (i.e. find maximal k). That is what Sage's perfect_power apparently does:
https://doc.sagemath.org/html/en/reference/rings_standard/sage/rings/integer.html#sage.rings.integer.Integer.perfect_power

This seems like the sort of situation where what is best in python-flint might be a bit different from what Flint itself does. I'm not sure what the performance implications would be though of calling fmpz_is_perfect_power in a loop (with successively smaller arguments) compared to just calling it once.

[oscarbenjamin]

Can you add tests in flint/tests/test_all.py?

Doctests are good for some simple things but generally it is better to have comprehensive tests including for edge cases outside of the doctests. We want full test coverage from the test suite and then doctests are good for showing tested examples in the documentation rather than actually serving as the test suite.

I'm not sure about these ones:

In [6]: flint.fmpz(-1).is_perfect_power()
Out[6]: True

In [7]: flint.fmpz(0).is_perfect_power()
Out[7]: True

In [8]: flint.fmpz(1).is_perfect_power()
Out[8]: True

Do we actually want that?

SymPy's perfect power rejects -1, 0 and 1. Wikipedia suggests that 0 and 1 are sometimes considered perfect powers but no mention of -1. It seems that flint's function like SymPy's accepts negative odd powers though so I guess it makes sense to have -1 if we also have e.g. -8.

[oscarbenjamin]

Okay, looks good.

We can merge this once tests are complete unless you want to add a .perfect_power() method as well.

[roos-j]

Thanks, I prefer to merge this now.

[oscarbenjamin]

Great. Thanks!