Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restore the standard behaviour of reversed(Lattice) #589

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Restore the standard behaviour of reversed(Lattice) #589

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 51 additions & 68 deletions pyat/at/lattice/lattice_object.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def __init__(self, *args,
``params_filter(params, ringparam_filter, *args)``
runs through ``ringparam_filter(params, *args)``, looks for
energy and periodicity if not yet defined.
"""
"""
if iterator is None:
arg1, = args or [[]] # accept 0 or 1 argument
if isinstance(arg1, Lattice):
Expand Down Expand Up @@ -271,9 +271,6 @@ def __iadd__(self, elems):
def __mul__(self, n):
return self.repeat(n)

def __reversed__(self):
return (el.swap_faces(copy=True) for el in self[::-1])

def _addition_filter(self, elems: Iterable[Element], copy_elements=False):
cavities = []
length = 0.0
Expand Down Expand Up @@ -309,14 +306,12 @@ def _addition_filter(self, elems: Iterable[Element], copy_elements=False):
def insert(self, idx: SupportsIndex, elem: Element, copy_elements=False):
r"""This method allow to insert an AT element in the lattice.

Parameters:
idx (SupportsIndex): index at which the lement is inserted
elem (Element): AT element to be inserted in the lattice

Keyword Arguments:
copy_elements(bool): Default :py:obj:`True`.
If :py:obj:`True` a deep copy of elem
is used.
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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't copy_elements a keyword argument?

If :py:obj:`True` a deep copy of elem
is used.
"""
# noinspection PyUnusedLocal
# scan the new element to update it
Expand All @@ -326,20 +321,18 @@ def insert(self, idx: SupportsIndex, elem: Element, copy_elements=False):

def extend(self, elems: Iterable[Element], copy_elements=False):
r"""This method adds all the elements of `elems` to the end of the
lattice. The behavior is the same as for a :py:obj:`list`
lattice. The behavior is the same as for a :py:obj:`list`

Equivalents syntaxes:
>>> ring.extend(elems)
>>> ring += elems

Parameters:
elem (Iterable[Element]): Sequence of AT elements to be
appended to the lattice
Equivalents syntaxes:
>>> ring.extend(elems)
>>> ring += elems

Keyword Arguments:
copy_elements(bool): Default :py:obj:`True`.
If :py:obj:`True` deep copies of each
element of elems are used
Parameters:
elems (Iterable[Element]): Sequence of AT elements to be
appended to the lattice
copy_elements(bool): Default :py:obj:`True`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

If :py:obj:`True` deep copies of each
element of elems are used
"""
if hasattr(self, '_energy'):
# When unpickling a Lattice, extend is called before the lattice
Expand All @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

copy_elements(bool): Default :py:obj:`True`.
If :py:obj:`True` a deep copy of elem
is used
Expand All @@ -369,24 +360,22 @@ def append(self, elem: Element, copy_elements=False):

def repeat(self, n: int, copy_elements=True):
r"""This method allows to repeat the lattice `n` times.
If `n` does not divide `ring.periodicity`, the new ring
periodicity is set to 1, otherwise it is et to
`ring.periodicity /= n`.

Equivalents syntaxes:
>>> newring = ring.repeat(n)
>>> newring = ring * n
If `n` does not divide `ring.periodicity`, the new ring
periodicity is set to 1, otherwise it is et to
`ring.periodicity /= n`.

Parameters:
n (int): number of repetition
Equivalents syntaxes:
>>> newring = ring.repeat(n)
>>> newring = ring * n

Keyword Arguments:
copy_elements(bool): Default :py:obj:`True`.
If :py:obj:`True` deepcopies of the
lattice are used for the repetition
Parameters:
n (int): number of repetition
copy_elements(bool): Default :py:obj:`True`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

If :py:obj:`True` deepcopies of the
lattice are used for the repetition

Returns:
newring (Lattice): the new repeated lattice
Returns:
newring (Lattice): the new repeated lattice
"""
def copy_fun(elem, copy):
if copy:
Expand Down Expand Up @@ -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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for these 2

deepcopies of the elements of lattices are used
copy(bool): Default :py:obj:`False`. If :py:obj:`True`
Expand All @@ -452,11 +439,7 @@ def reverse(self, copy=False):
r"""Reverse the order of the lattice and swapt the faces
of elements. Alignment errors are not swapped


Usage:
>>> newring = ring.reverse(copy=True)

Keyword Arguments:
Parameters:
copy(bool): Default :py:obj:`False`. If :py:obj:`True`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@lfarv lfarv Apr 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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…

the lattice is modified in place.
Oterwise a new Lattice object is returned
Expand All @@ -465,15 +448,17 @@ def reverse(self, copy=False):
lattice(Lattice): reversed Lattice, if `copy==True` the
new lattice object is returned
otherwise None
Example:
>>> newring = ring.reverse(copy=True)
"""
elems = (el.swap_faces(copy=True) for el in reversed(self))
if copy:
elems = (el.swap_faces(copy=copy) for el in self[::-1])
return Lattice(elem_generator, elems, iterator=self.attrs_filter)
else:
reversed_list = list(reversed(self))
reversed_list = list(elems)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

self[:] = reversed_list

def develop(self) -> "Lattice":
def develop(self) -> Lattice:
"""Develop a periodical lattice by repeating its elements
*self.periodicity* times

Expand Down Expand Up @@ -503,7 +488,7 @@ def extattr(d):
res.update((k, v) for k, v in vrs.items() if not k.startswith('_'))
return res

def rotate(self, n: int) -> "Lattice":
def rotate(self, n: int) -> Lattice:
"""Return a new lattice rotated left by n elements"""
if len(self) == 0:
return self.copy()
Expand All @@ -522,22 +507,20 @@ def update(self, *args, **kwargs) -> None:
for (key, value) in attrs.items():
setattr(self, key, value)

def copy(self) -> "Lattice":
def copy(self) -> Lattice:
"""Returns a shallow copy of the lattice"""
return copy.copy(self)

def deepcopy(self) -> "Lattice":
def deepcopy(self) -> Lattice:
"""Returns a deep copy of the lattice"""
return copy.deepcopy(self)

def slice_elements(self, refpts: Refpts, slices: Optional[int] = 1) \
-> "Lattice":
def slice_elements(self, refpts: Refpts, slices: int = 1) -> Lattice:
"""Create a new lattice by slicing the elements at refpts
Parameters:
refpts: element selector

Keyword arguments:
slices=1: Number of slices in the specified range. Ignored if
Parameters:
refpts: Element selector
slices: Number of slices in the specified range. Ignored if
size is specified. Default: no slicing

Returns:
Expand All @@ -546,7 +529,7 @@ def slice_elements(self, refpts: Refpts, slices: Optional[int] = 1) \
def slice_generator(_):
check = get_bool_index(self, refpts)
for el, ok in zip(self, check):
if ok and (slices>1):
if ok and (slices > 1):
frac = numpy.ones(slices) / slices
for elem in el.divide(frac):
yield elem
Expand All @@ -556,7 +539,7 @@ def slice_generator(_):
return Lattice(slice_generator, iterator=self.attrs_filter)

def slice(self, size: Optional[float] = None, slices: Optional[int] = 1) \
-> "Lattice":
-> Lattice:
"""Create a new lattice by slicing the range of interest into small
elements

Expand Down Expand Up @@ -1043,7 +1026,7 @@ def passm(key, eltype, def_pass):
lattice_modify()

# noinspection PyShadowingNames,PyIncorrectDocstring
def enable_6d(self, *args, **kwargs) -> Optional["Lattice"]:
def enable_6d(self, *args, **kwargs) -> Optional[Lattice]:
# noinspection PyUnresolvedReferences
r"""
enable_6d(elem_class[, elem_class]..., copy=False)
Expand Down Expand Up @@ -1142,7 +1125,7 @@ def enable_6d(self, *args, **kwargs) -> Optional["Lattice"]:
return self._set_6d(True, *args, **kwargs)

# noinspection PyShadowingNames,PyIncorrectDocstring
def disable_6d(self, *args, **kwargs) -> Optional["Lattice"]:
def disable_6d(self, *args, **kwargs) -> Optional[Lattice]:
# noinspection PyUnresolvedReferences
r"""
disable_6d(elem_class[, elem_class]... , copy=False)
Expand Down Expand Up @@ -1298,7 +1281,7 @@ def next_mk():
return Lattice(sbreak_iterator, iter_mk,
iterator=self.attrs_filter, **kwargs)

def reduce(self, **kwargs) -> "Lattice":
def reduce(self, **kwargs) -> Lattice:
"""Removes all elements with an ``IdentityPass`` PassMethod and merges
compatible consecutive elements.

Expand Down Expand Up @@ -1335,7 +1318,7 @@ def reduce_filter(_, itelem):
return Lattice(reduce_filter, self.select(kp | keep),
iterator=self.attrs_filter, **kwargs)

def replace(self, refpts: Refpts, **kwargs) -> "Lattice":
def replace(self, refpts: Refpts, **kwargs) -> Lattice:
"""Return a shallow copy of the lattice replacing the selected
elements by a deep copy

Expand All @@ -1348,7 +1331,7 @@ def replace(self, refpts: Refpts, **kwargs) -> "Lattice":
iterator=self.attrs_filter, **kwargs)

# Obsolete methods kept for compatibility
def radiation_on(self, *args, **kwargs) -> Optional["Lattice"]:
def radiation_on(self, *args, **kwargs) -> Optional[Lattice]:
"""Obsolete. Turn longitudinal motion on

The function name is misleading, since the function deals with
Expand All @@ -1364,7 +1347,7 @@ def radiation_on(self, *args, **kwargs) -> Optional["Lattice"]:
zip(('cavity_pass', 'dipole_pass', 'quadrupole_pass'), args))
return self._set_6d(True, **kwargs)

def radiation_off(self, *args, **kwargs) -> Optional["Lattice"]:
def radiation_off(self, *args, **kwargs) -> Optional[Lattice]:
"""Obsolete. Turn longitudinal motion off

The function name is misleading, since the function deals with
Expand Down