some cleanup in quadratic forms

[fchapoton]

some pyflakes, pep, and code cleanup in just a few files there

CC: @tscrim @simonbrandhorst

Component: quadratic forms

Author: Frédéric Chapoton

Branch/Commit: c0fc402

Reviewer: Travis Scrimshaw

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

[fchapoton]

Branch: u/chapoton/27726

[fchapoton]

Commit: 63f4059

[fchapoton]

New commits:

63f4059

some cleanup in quadratic forms

[fchapoton]

comment:2

green bot, please review

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:3

I am not quite in favor of these changes:

-#from sage.quadratic_forms.quadratic_form import QuadraticForm    ## This creates a circular import! =(
 
 
-####################################################################################
-## Routines used for understanding p-neighbors, and computing classes in a genus. ##
-####################################################################################
+# ######################################################################
+# Routines used for understanding p-neighbors and computing classes in a genus.
+# ######################################################################

The first one deserves to be there as a warning for future code. For the latter, the first looks better visually IMO. However, that one is clearly bikeshedding, so I won't push it.

-    ## Find a (dual) vector w with B(v,w) != 0 (mod p)
-    v_dual = B2 * vector(v)     ## We want the dot product with this to not be divisible by 2*p.
+    # Find a (dual) vector w with B(v,w) != 0 (mod p)
+    v_dual = B2 * vector(v)
+    # We want the dot product with this to not be divisible by 2*p.

For this change, I feel the comment is better served before the v_dual line if you do not want it on the same line.

     if hasattr(self, "__split_local_cover"):
         if is_QuadraticForm(self.__split_local_cover):  ## Here the computation has been done.
             return self.__split_local_cover
-        elif isinstance(self.__split_local_cover, Integer):    ## Here it indexes the values already tried!
+        elif self.__split_local_cover in ZZ:    ## Here it indexes the values already tried!
             current_length = self.__split_local_cover + 1
             Length_Max = current_length + 5
     else:

I am slightly worried about this change as ZZ and QQ elements are slightly different and this can matter. I don't think so, but I am cc-ing Simon to see if he agrees that this change is completely safe (and to look over the other changes in case he has some other comments).

[simonbrandhorst]

comment:4

grepping __split_local_cover
shows that is is either a quadratic form
or an Integer. The change mentioned should be safe.

Thanks for the cleanup.
Note that there is still a lot of broken/incomplete code around
in the quadratic forms folder.

[sagetrac-git]

Changed commit from 63f4059 to c0fc402

[sagetrac-git]

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

c0fc402

trac 27726 some details

[fchapoton]

comment:6

some fixes as suggested

[tscrim]

comment:7

Thanks. LGTM.

[vbraun]

Changed branch from u/chapoton/27726 to c0fc402