Delete _check_rank_two method for Drinfeld modules

[kryzar]

This method does nothing if the Drinfeld module has rank two, and raises an exception otherwise. It was introduced in the first version of the implementation, when some methods were only available for the rank two case (e.g. frobenius_trace, frobenius_norm, etc). A grep reveals that the method is never called, which makes it useless. Therefore, I propose to remove it.

[github-actions]

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

[DavidAyotte]

I agree about this change, however you cannot remove it right away. It should first go through the deprecation process of SageMath (see the developer's manual, especially the "Any other case"). This means that the method should display a deprecation message and this message should be there for at least a whole year after the first stable release where the deprecation was implemented.

This is mainly because the method is still accessible by the user (even if it's not visible in the manual):

sage: A = GF(5)['T']; K.<T> = Frac(A)
sage: phi = DrinfeldModule(A, [K.gen(), 1])
sage: phi._check_rank_two()
Traceback (most recent call last):
...
NotImplementedError: rank must be 2

Since there is no way of knowing if a silly user has already used it, we need to warn them that the method will be removed. In the deprecation message, you can suggest an alternative.

[kryzar]

I agree about this change, however you cannot remove it right away. It should first go through the deprecation process of SageMath (see the developer's manual, especially the "Any other case"). This means that the method should display a deprecation message and this message should be there for at least a whole year after the first stable release where the deprecation was implemented.

This is mainly because the method is still accessible by the user (even if it's not visible in the manual):

sage: A = GF(5)['T']; K.<T> = Frac(A)
sage: phi = DrinfeldModule(A, [K.gen(), 1])
sage: phi._check_rank_two()
Traceback (most recent call last):
...
NotImplementedError: rank must be 2

Since there is no way of knowing if a silly user has already used it, we need to warn them that the method will be removed. In the deprecation message, you can suggest an alternative.

Right, thanks. But does that also apply to methods whose name starts with an underscore?

[DavidAyotte]

Right, thanks. But does that also apply to methods whose name starts with an underscore?

I have no idea, but I think it's safer (mais plus chiant) to let it go through the deprecation process.

[grhkm21]

Nope, underscore methods are meant for development and don't need to go through deprecation process, see this comment

[DavidAyotte]

Nope, underscore methods are meant for development and don't need to go through deprecation process, see this comment

hmm I'm not entirely convinced by this since you are just cherry-picking a GitHub comment and I still think it is safer to deprecate any change to a method (internal or not), but I don't have much difficulty in believing that no one ever used the _check_rank_two method.

[kryzar]

OK. I did not find anything more official. I think that a dev. should be allowed to remove any private or semi-private callable without any deprecation, but I agree with David that the development guide is ambiguous. We can put a deprecation warning and bring that issue to some other members of the community (in a separate issue, and not on this PR!), as to have a more official stance, and put it in the manual.

Note also that the dev. guide that the number of the deprecation refers to a PR:

Each deprecation warning contains the number of the GitHub PR that defines it. We use 666 in the examples below. For each entry, consult the function’s documentation for more information on its behaviour and optional arguments.

while in reality it refers to an issue:

sage: Fq = GF(2)
sage: K.<z> = Fq.extension(2)
sage: A.<T> = Fq[]
sage: phi = DrinfeldModule(A, [z, 0, 1])
sage: phi._check_rank_two()
<ipython-input-5-5cc38fb8d306>:1: DeprecationWarning: This semi-private method should not be used. It will be removed.
See https://github.com/sagemath/sage/issues/38199 for details.
  phi._check_rank_two()
sage:

This can be fixed, but is probably tedious (lots of things to replace in $SAGE_INSTALL/src/sage/misc/superseded.py, which I don't have the time to do now or anytime soon).

[kryzar]

Opened #38211.

[xcaruso]

After the discussion of issue #38211, I think that you can remove safely the method without any deprecation.

[DavidAyotte]

LGTM

[kryzar]

(It seems that the failing doctests are not related to Drinfeld modules: https://github.com/sagemath/sage/actions/runs/9669672715/job/26676699605?pr=38199#step:10:9571)