move ProductTree and prod_with_derivative() to sage.rings.generic

[yyyyx4]

The class ProductTree and the function prod_with_derivative() were introduced in #34303. Both are fully generic in principle, but they remained in hom_velusqrt.py in the heat of the moment.

We should move them to a more suitable place; it seems sage.rings.generic is an appropriate choice. Slight tweaks to ProductTree while we're at it.

CC: @jhpalmieri

Component: algebra

Author: Lorenz Panny

Branch/Commit: c3dcb9b

Reviewer: Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/34791

[yyyyx4]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-The class `ProductTree` and the function `prod_with_derivative()` were introduced in #34303. Both are fully generic in principle, but they were included in `hom_velusqrt.py` in the heat of the moment.
+The class `ProductTree` and the function `prod_with_derivative()` were introduced in #34303. Both are fully generic in principle, but they remained in `hom_velusqrt.py` in the heat of the moment.
 
-We should move them to a more suitable place; it seems `sage.arith.misc` is an appropriate choice. While we're at it, slight tweaks to `ProductTree`, and I believe importing it by default is a good idea to increase visibility and ease of use.
+We should move them to a more suitable place; it seems `sage.arith.misc` is an appropriate choice. Slight tweaks to `ProductTree` while we're at it, and I believe importing it by default is a good idea to increase visibility and ease of use.
 

[mezzarobba]

comment:2

I think ProductTree could even go to its own file, since it may well be extended or ported to cython. Also, most of misc.py has a more number-theoretic flavor. (But that's just a suggestion and I'm fine with misc.py too.)

[yyyyx4]

comment:3

Currently sage.misc.misc_c contains the highly related prod() function. I think this could also be in a location such as sage.rings.generic (which doesn't currently exist), similar to how sage.groups.generic contains generic-group algorithms. Personally I don't have a preference.

[kwankyu]

comment:4

I believe importing it by default is a good idea to increase visibility and ease of use.

-1. I don't think it has so general use. But I would hear John.

[jhpalmieri]

comment:5

First, especially if there might be expansions to ProductTree, it might very well make sense to put it in its own file. Second, regarding increased visibility, I would say no right now. For example I look at the documentation and I can't tell what it does. (There is one line of description, "A simple product tree," there is one line saying what the inputs are, and that's pretty much the extent of the documentation. I think it should have much more.) You should make sure it's in the reference manual, and if you think it's particularly useful, write a thematic tutorial about it and add that to the documentation — that's a good way to increase visibility.

[sagetrac-git]

Changed commit from 3ff3cdf to 8e6d94c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

8e6d94c

Revert "import ProductTree with sage.arith.all"

[kwankyu]

comment:7

Replying to John Palmieri:

For example I look at the documentation and I can't tell what it does. (There is one line of description, "A simple product tree," there is one line saying what the inputs are, and that's pretty much the extent of the documentation. I think it should have much more.)

+1. In the ProductTree class docstring, there is no description about a product tree. It may be described in terms of leaves, layers, and the root in a couple of sentences.

Instead of a sequence of (which sounds mathematical), a list of would be pythonic, and avoid an unnecessary confusion.

The ALGORITHM section of prod_with_derivative just describes the source code. Thus it is too much detailed but useless for a user. I suggest to remove it.

The paragraph The main reason for this function ... may be moved up and replace the aforementioned ALGORITHM section.

[yyyyx4]

comment:8

Thanks for the feedback, everyone. I'll work on the documentation.

Replying to Kwankyu Lee:

Instead of a sequence of (which sounds mathematical), a list of would be pythonic, and avoid an unnecessary confusion.

But in Python "a list" is much more specific than "a sequence"! For example, a tuple is a sequence but not a list. Indeed, the Python documentation does have a concept of "sequence": https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range

The paragraph The main reason for this function ... may be moved up and replace the aforementioned ALGORITHM section.

Are you suggesting to move it outside of theEXAMPLES block? Or just swap the two examples?