py3: some details in polyhedron

[chapoton]

New commits:

​5a6d66a

some py3 details in polyhedron/

[git]

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

​1264a7a

one detail

[jhpalmieri]

I don't know if this is the ideal fix, but it works for me.

[jhpalmieri]

Actually, I'm going to try another fix first:

src/sage/geometry/polyhedron/backend_cdd.py

@@ -218 +218 @@
         cddout = cddout.splitlines()
 
         def parse_indices(count, cdd_indices, cdd_indices_to_sage_indices=None):
-            cdd_indices = list(map(int, cdd_indices))
+            cdd_indices = [int(_) for _ in cdd_indices]
             if cdd_indices_to_sage_indices is None:
                 cdd_indices_to_sage_indices = {i:i-1 for i in cdd_indices}
             if count < 0:
@@ -247 +247 @@
             assert self.ambient_dim() == dimension - 1, "Unexpected ambient dimension"
             assert len(data) == count, "Unexpected number of lines"
             for i, line in enumerate(data):
-                coefficients = list(map(self.base_ring(), line))
+                coefficients = [self.base_ring()(x) for x in line]
                 if coefficients[0] != 0 and all(e == 0 for e in coefficients[1:]):
                     # cddlib sometimes includes an implicit plane at infinity: 1 0 0 ... 0
                     # We do not care about this entry.

[jhpalmieri]

This new version works for me and I prefer it: there is no reason to use map if we want a list and can use list comprehension instead. So I propose we use this instead.

[jhpalmieri]

Of course I proposed the first change, too, so sorry about that. I was just trying to get the PDF docs to build, not thinking too clearly about the right way to do things.

---

New commits:

​e57d3df

trac 26035: use list comprehension instead of list(map(...))

[chapoton]

ok for list comprehension.

but please do not use underscore _, but a normal variable name.

[git]

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

​8c1c081

trac 26035: Don't use _ in list comprehension. Convert 'data' to a list

[jhpalmieri]

Okay, no _. Also, in moving the patch from #25840 to here, the change data = list(data) got moved to the wrong place. That's fixed, so the plot which was problematic before in Python 3 now works. If you want to test, it's this one:

sage: M = matrix([[0., -0.556793440452, 0.19694925177, -0.19694925177, 0.0805477263944, -0.385290876171, 0., 0.3852
....: 90876171], [0.180913155536, 0., 0.160212955043, 0.160212955043, 0., 0.0990170516545, 0.766360424875, 0.099017
....: 0516545],[0.338261212718, 0, 0, -0.338261212718, 0.672816364803, 0.171502564281, 0, -0.171502564281]])
sage: L = RootSystem(["E",8]).ambient_space()
sage: L.plot(roots="all", reflection_hyperplanes=False, projection=lambda v: M*vector(v), labels=False)

[chapoton]

good for me if the bot is morally green

[jhpalmieri]

Great, thanks (bot was green).

[tscrim]

Quick little micro-optimization: Instead of

coefficients = [self.base_ring()(x) for x in line]

which repeatedly called the base_ring(), I would move that outside of the bigger for loop. However, you can do that on a later ticket. :)

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​b357154

trac 26035: minor optimization

[jhpalmieri]

We can deal with that now.

[tscrim]

Thank you.