GitLab

The Rc model parameter of surfaces (i.e. Mirror and Beamsplitter) sometimes returns a single number and sometimes a tuple, depending on whether Rcx and Rcy are the same within tolerance:

    @property
    def Rc(self):
        """The radius of curvature of the mirror in metres. If the
        mirror is astigmatic this is a tuple of the tangential and
        sagittal radii of curvature.

        :getter: Returns the radius of curvature of the mirror in metres (a
                 tuple of values if the mirror is astigmatic).

        :setter: Sets the radius of curvature.

        Examples
        --------
        The following sets the radii of curvature of an object `obj`, which
        is a sub-instance of `Surface`, in both directions to 2.5 m:

        >>> obj.Rc = 2.5

        Whilst this would set the radius of curvature in the x-direction (tangential
        plane) to 2.5 and the radius of curvature in the y-direction (sagittal plane)
        to 2.7:

        >>> obj.Rc = (2.5, 2.7)
        """
        if np.isclose(self.Rcx.value, self.Rcy.value):
            return self.Rcx
        return (self.Rcx, self.Rcy)

It's worth thinking about whether this is the way this should work. This is potentially confusing to users, for instance if they write some code and test that it works for the Rc values they try but later find it doesn't work when some other Rc values are used. You don't want some long running code to fail because of an unexpectedly different data type like this. This also causes extra work for our tests because we have to check the data type before comparing.

I like that the property lets the Rc value get set either as single number or a tuple with two representing x and y, but maybe it should return a tuple of x and y values in all circumstances. If users really want a particular axis they can use Rcx and Rcy.

The same issue applies to some of Cavity's properties too.