Shi arrangement for other types

[sagetrac-etzanaki]

m-extended Shi arrangement defined for any finite crystallograpic root system

CC: @VivianePons @tscrim

Component: combinatorics

Keywords: days98

Author: Eleni Tzanaki, Viviane Pons

Branch/Commit: b176a2a

Reviewer: Frédéric Chapoton, Travis Scrimshaw

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

[sagetrac-etzanaki]

Changed keywords from none to #days98

[tscrim]

Changed keywords from #days98 to days98

[sagetrac-etzanaki]

Branch: u/etzanaki/shi_arrangement_for_other_types

[sagetrac-etzanaki]

Commit: 0924213

[sagetrac-etzanaki]

Author: Eleni Tzanaki

[sagetrac-etzanaki]

New commits:

0924213

Adding cartan type and m-extended Shi arrangments

[VivianePons]

Changed branch from u/etzanaki/shi_arrangement_for_other_types to u/VivianePons/shi_arrangement_for_other_types

[tscrim]

comment:7

Some PEP8 and other misc small comments:

-    def Shi(self, data, K=QQ, names=None, m = 1):
+    def Shi(self, data, K=QQ, names=None, m=1):

         The characteristic polynomial is pre-computed using the results of
-         [Ath1996]_ .
+        [Ath1996]_.

         if data in NN:
-            cartan_type = CartanType(["A",data -1])
+            cartan_type = CartanType(['A', data-1])

         for a in PR:
             for const in range(-m+1,m+1):
-                hyperplanes.append(sum(a[j]*x[j] for j in range(d
-                ))-const)
+                hyperplanes.append(sum(a[j]*x[j] for j in range(d))-const)

(the 80 char/line is much more of a guideline and you should never sacrifice readability for it IMO).

-        charpoly = (x**(d-n))*(x-m*h)**n
+        charpoly = x**(d-n) * (x - m*h)**n

It would also be nice to remove those added blanklines at the end of the file.

---

New commits:

411f5e1

getting rid of trailing white spaces and some mysterious "non ascii character"

[tscrim]

Changed commit from 0924213 to 411f5e1

[sagetrac-git]

Changed commit from 411f5e1 to f1bd24e

[sagetrac-git]

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

f1bd24e

PEP8 small changes and removing blanklines

[VivianePons]

comment:9

It looks like the bot is happy. I have fixed the small things pointed by Travis.

Eleni wrote the initial code and I helped with the design and details. The code extends the previous functionalities of the function. The previous usage is still valid and works exactly the same (we haven't changed those tests) and now we have more options (with Cartan types and m parameter), all documented. Eleni checked the math quite carefully.

So Travis, I let you have a look and hopefully, we can have a positive review soon.

[fchapoton]

comment:10

• avoid using unicode "–" in the references, just use - (minus)

• in the doctests, write h = hyperplane, namely add space around =

• do not repeat the "EXAMPLES::"

otherwise, looks good

[fchapoton]

comment:11

and this line is useless, as told by the patchbot:

R = RootSystem(cartan_type)

[sagetrac-git]

Changed commit from f1bd24e to b176a2a

[sagetrac-git]

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

b176a2a

Small fixes following comments

[VivianePons]

comment:13

All done!

[fchapoton]

Reviewer: Frédéric Chapoton

[fchapoton]

comment:14

ok, thx

[tscrim]

Changed author from Eleni Tzanaki to Eleni Tzanaki, Viviane Pons

[tscrim]

comment:15

Viviane, I added you as an author based on comment:9. If you feel you closer to a reviewer, feel free to move yourself over.

[tscrim]

Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Travis Scrimshaw

[vbraun]

Changed branch from u/VivianePons/shi_arrangement_for_other_types to b176a2a