pep cleanup for the quivers folder

[fchapoton]

mainly cosmetic changes in code presentation

CC: @tscrim

Component: algebra

Author: Frédéric Chapoton

Branch/Commit: e7695d3

Reviewer: Travis Scrimshaw

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

[fchapoton]

Branch: u/chapoton/28461

[fchapoton]

Commit: 08cc7dc

[fchapoton]

New commits:

08cc7dc

pep cleanup for the quivers folder

[fchapoton]

comment:2

green bot, please review

[tscrim]

comment:3

Is there a reason why you didn't also change this to a dict comprehension?

-        elems = dict((v, self._left_action_mats[edge][v]*element._elems[v]) for v in self._quiver)
+        elems = dict((v, self._left_action_mats[edge][v]*element._elems[v])
+                     for v in self._quiver)

In this change:

-        result = parent()   # this must not be the cached parent.zero(),
-                             # since otherwise it gets changed in place!!
+        result = parent()
+        # this must not be the cached parent.zero(),
+        # since otherwise it gets changed in place!!

I think it would be better for the comment to go first. Also, in principle, the result of parent() could use the cached zero method since we assume elements are immutable. So I think it would be better (and faster) to explicitly construct the zero element.

[sagetrac-git]

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

a433abd	Merge branch 'u/chapoton/28461' in 8.9.b9
e7695d3	trac 28461 fix some details

[sagetrac-git]

Changed commit from 08cc7dc to e7695d3

[fchapoton]

comment:5

I have made the trivial changes that you suggested.

Concerning the zero, it seems that there is really a problem. This looks serious enough that it should be kept for a dedicated ticket. Moreover, I am not sure that the coercion framework is used correctly, as this happens inside a double underscore method __mul__.

[tscrim]

comment:6

Replying to @fchapoton:

I have made the trivial changes that you suggested.

Thank you.

Concerning the zero, it seems that there is really a problem. This looks serious enough that it should be kept for a dedicated ticket. Moreover, I am not sure that the coercion framework is used correctly, as this happens inside a double underscore method __mul__.

Sounds good. cc me on that and I can do the review, or I can make the change tomorrow if you have not started on it already.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

Changed branch from u/chapoton/28461 to e7695d3