# Changes between Initial Version and Version 6 of Ticket #29886

Ignore:
Timestamp:
Jun 18, 2020, 3:44:48 PM (2 years ago)
Comment:

Use `is None` to test if something is `None`:

```- if g == None:
+ if g is None:
```

I would also use `f=None` to mean `f` is the identity -- giving the standard "symmetric" matrices:

```if f is None:
f = lambda x: x
```

I would write the input as:

```- ``f`` -- function or ``None`` (optional; default: ``None``);
if ``None``, use the identity function

- ``g`` -- function or ``None`` (optional; default: ``None``);
if ``None``, use the same function as for ``f``
```

Need to decide what symmetric should mean

• Among possible definitions:
• subdiagonal entries are images by f of the corresponding superdiagonal entries, i.e. for any `i < j` we have `A[j,i] = f(A[i,j])`
• nondiagonal entries are images by f of their mirror entries, i.e. for any `i != j` we have `A[j,i] = f(A[i,j])`
• These are the same if `f` is an involution; if that is assumed it needs to be said
• Accordingly, do all or only half the checks

I would avoid formulas in the docstring's first sentence and clarify in a second sentence:

```Return a generator of all matrices in this matrix space
that are "symmetric" as defined by `f` and `g`.

This means subdiagonal entries are images by ``f``
of the corresponding superdiagonal entries, i.e.
for any ``i < j`` we have ``A[j,i] = f(A[i,j])``,
and diagonal elements are stable by ``g`` ,
i.e. for any ``i`` we have ``A[i,i] = g(A[i,i])``.

The default value of ``None`` for ``f`` means
``f`` is the identity, and the default value
of ``None`` for ``g`` means ``g`` is ``f``.
```

Need `::` here to start the doctest block:

```Skew-symmetric matrices::
```

I would use `v` rather than `iv` for the loop variable.

Comments could be more concise:

```- # If the number of entries is zero, then just
- # yield the empty matrix in that case and return
+ # If no entries, yield empty matrix and return
if number_of_entries == 0:
yield self(0)
return
```
```- # When the base ring is not finite, then we should go
- # through and yield the matrices by "weight", which is
- # the total number of iterations that need to be done
- # on the base ring to reach the matrix.
+ # For infinite rings, yield matrices by "weight",
+ # i.e. by number of iterations needed to reach them.
```
```- # In the finite case, we do a similar thing except that
- # instead of checking if the diagonal is correct after creating the vector
- # we can select all possible diagonal elements a priori
+ For finite rings, allow all ring elements on the diagonal a priori.
```

### Legend:

Unmodified
• Property Status changed from `new` to `needs_review`
• Property Commit changed from `a24e6a07971f44715896523d15b8c61633ef9e52` to `9b5145b551a552ece2f31c0cf6a42b7c10d08f89`
• Property Summary changed from `Generator of symmetric matrices.` to `Add generator of symmetric matrices`
• Property Authors changed from `gh-Ivo-Maffei` to `Ivo Maffei`