Restore the standard behaviour of reversed(Lattice) #589
Conversation
| Parameters: | ||
| idx (SupportsIndex): index at which the lement is inserted | ||
| elem (Element): AT element to be inserted in the lattice | ||
| copy_elements(bool): Default :py:obj:`True`. |
There was a problem hiding this comment.
Isn't copy_elements a keyword argument?
| Parameters: | ||
| elems (Iterable[Element]): Sequence of AT elements to be | ||
| appended to the lattice | ||
| copy_elements(bool): Default :py:obj:`True`. |
| @@ -359,8 +352,6 @@ def append(self, elem: Element, copy_elements=False): | |||
|
|
|||
| Parameters: | |||
| elem (Element): AT element to be appended to the lattice | |||
|
|
|||
| Keyword Arguments: | |||
| lattice are used for the repetition | ||
| Parameters: | ||
| n (int): number of repetition | ||
| copy_elements(bool): Default :py:obj:`True`. |
| @@ -427,8 +416,6 @@ def concatenate(self, *lattices: Iterable[Element], | |||
| lattices: :py:obj:`Iterables[Element]` to be concatenanted | |||
| to the Lattice, several lattices are allowed | |||
| (see example) | |||
|
|
|||
| Keyword Arguments: | |||
| copy_elements(bool): Default :py:obj:`False`. If :py:obj:`True` | |||
There was a problem hiding this comment.
Same comment for these 2
| >>> newring = ring.reverse(copy=True) | ||
|
|
||
| Keyword Arguments: | ||
| Parameters: | ||
| copy(bool): Default :py:obj:`False`. If :py:obj:`True` |
There was a problem hiding this comment.
I'd like to keep them as keywords, but I get lots of warnings in PyCharm about arguments missing in docstrings: apparently for Sphinx, every argument mentioned in he parameter list must be mentioned in the "Args:" section, "Keyword Args:" is reserved for those in kwargs…
I can live with warnings, but they may hide important ones… I like to keep the "Problems" tab empty.
There was a problem hiding this comment.
By the way, I got a similar problem with the "Usage:" section: "Usage:" is not a valid section for sphinx, and it was not formatted properly as python code in the output. That's why I converted it to an "Example:" section…
| return Lattice(elem_generator, elems, iterator=self.attrs_filter) | ||
| else: | ||
| reversed_list = list(reversed(self)) | ||
| reversed_list = list(elems) |
There was a problem hiding this comment.
Doing this you copy the elements. Is it possible and would it make sense to return the elements themselves by using copy=copy in the elems generator?
There was a problem hiding this comment.
No copy is a problem: if the same element is used twice in the ring, it will be swapped twice, and the second one in the reversed lattice will be wrong.
|
Ok thanks for the clarification, all good for me! |
This PR restores the standard behaviour of the
reversed()iterator forLatticeobjects: the behaviour introduced in #573 could be confusing, since it was iterating on copies of the elements instead of element themselves.As a consequence, the only way to build reversed lattices by swapping entrance and exit faces is to use the
Lattice.reversed()method.