pep8 cleanup for one file in quadratic_forms

[fchapoton]

also a few code and doc details

CC: @tscrim @kwankyu @slel @dcoudert

Component: quadratic forms

Author: Frédéric Chapoton

Branch/Commit: 15f50e0

Reviewer: Kwankyu Lee

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

[fchapoton]

Commit: f0be16d

[fchapoton]

Branch: u/chapoton/34304

[fchapoton]

New commits:

f0be16d

pep8 for one file in quadratic_forms (renamed file)

[mkoeppe]

comment:2

diff --git a/src/sage/quadratic_forms/quadratic_form.py b/src/sage/quadratic_forms/quadratic_form.py
index f5ac84b..e76e1ab 100644
--- a/src/sage/quadratic_forms/quadratic_form.py
+++ b/src/sage/quadratic_forms/quadratic_form.py
@@ -453,13 +453,13 @@ class QuadraticForm(SageObject):
...
+    from sage.quadratic_forms.local_representation_conditions import (
+        local_representation_conditions,

I think it's problematic that the imported name local_representation_conditions shadows the module name sage.quadratic_forms.local_representation_conditions

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ff822a6

pep8 for one file in quadratic_forms (renamed file)

[sagetrac-git]

Changed commit from f0be16d to ff822a6

[fchapoton]

comment:4

ok, right. Now with a shorter filename

[slel]

comment:5

The .. TODO:: block seems indented by 3 spaces instead of 4.

[sagetrac-git]

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

9aa09a1

indent TODO

[sagetrac-git]

Changed commit from ff822a6 to 9aa09a1

[fchapoton]

comment:7

voila

[fchapoton]

comment:8

review, please?

[kwankyu]

comment:9

You renamed sage.quadratic_forms.quadratic_form__local_representation_conditions to sage.quadratic_forms.local_representation.

There are many sage.quadratic_forms.quadratic_form__xxx modules in the same package. Hence your change make only one of those modules out of the naming pattern.

Why only one module? Do you plan to change the name of other modules too?

If you plan to change them all, how about the pattern sage.quadratic_forms._local_representation_conditions?

And I think we should keep the original name local_representation_conditions as the original author intended.

[fchapoton]

comment:10

I changed only one filename to avoid controversy.

My first choice for the shorter name was indeed local_representation_conditions but objected in comment:2 so I picked a shorter one.

There are also many files in the folder that already do not follow the pattern quadratic_form__xxx so I am not breaking the pattern.

[kwankyu]

comment:11

Replying to @fchapoton:

My first choice for the shorter name was indeed local_representation_conditions but objected in comment:2 so I picked a shorter one.

I understood. But dropping a word has danger of changing the meaning. So instead of dropping a part, I suggested to add the prefix _. Then this pattern may be applied to other similar files later...

[fchapoton]

comment:12

Sorry, using the prefix _ for a module name does not look like a good idea.

Let's try to choose another name, that would suit everybody..

Maybe local_representation_criteria ?

[kwankyu]

comment:13

If the aim is to have a short name, how about

qf__local_representation_conditions

?

[kwankyu]

comment:14

I think these modules quadratic_form__xxx are special in that they were created to provide class method definitions in separate files. So a slightly non-conventional naming is okay if it could satisfy the style checker. Like the prefix qf__ :)

[mkoeppe]

comment:15

It may make sense to change it back to the original for this ticket, and open a separate ticket for doing all renames simultaneously.

[tscrim]

comment:16

My 2 cents.

I agree with Matthias in comment:15; we should rename all of them on one (separate) ticket.

I am +1 on removing quadratic_form__ from all of the filenames as they are redundant with the folder name.

I am not so worried about comment:2; we have ways of resolving such conflicts with, e.g., import foo as bar. Yet, it would be good to avoid that for us humans who get confused.

[kwankyu]

comment:17

Replying to @mkoeppe:

It may make sense to change it back to the original for this ticket, and open a separate ticket for doing all renames simultaneously.

+1.

[sagetrac-git]

Changed commit from 9aa09a1 to 15f50e0

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

15f50e0

pep8 for one file in quadratic_forms

[fchapoton]

comment:19

ok, so here is a branch not renaming the modified file

[kwankyu]

Reviewer: Kwankyu Lee

[kwankyu]

comment:20

LGTM.

[vbraun]

Changed branch from u/chapoton/34304 to 15f50e0