applied suggestions from code review

docs/source/usersguide/install.rst

@@ -404,7 +404,7 @@ to install the Python package in :ref:`"editable" mode <devguide_editable>`.
 Prerequisites
 -------------
 
-The Python API works with Python 3.4+. In addition to Python itself, the API
+The Python API works with Python 3.5+. In addition to Python itself, the API
 relies on a number of third-party packages. All prerequisites can be installed
 using Conda_ (recommended), pip_, or through the package manager in most Linux
 distributions. To run simulations in parallel using MPI, it is recommended to

openmc/model/funcs.py

@@ -373,52 +373,7 @@ def get_hexagonal_prism(*args, **kwargs):
     return hexagonal_prism(*args, **kwargs)
 
 
-def cylinder_from_points(p1, p2, r, **kwargs):
-    """Return cylinder defined by two points passing through its center.
-
-    Parameters
-    ----------
-    p1, p2 : 3-tuples
-        Coordinates of two points that pass through the center of the cylinder
-    r : float
-        Radius of the cylinder
-    kwargs : dict
-        Keyword arguments passed to the :class:`openmc.Quadric` constructor
-
-    Returns
-    -------
-    openmc.Quadric
-        Quadric surface representing the cylinder.
-
-    """
-    # Get x, y, z coordinates of two points
-    x1, y1, z1 = p1
-    x2, y2, z2 = p2
-
-    # Define intermediate terms
-    dx = x2 - x1
-    dy = y2 - y1
-    dz = z2 - z1
-    cx = y1*z2 - y2*z1
-    cy = x2*z1 - x1*z2
-    cz = x1*y2 - x2*y1
-
-    # Given p=(x,y,z), p1=(x1, y1, z1), p2=(x2, y2, z2), the equation for the
-    # cylinder can be derived as r = |(p - p1) ⨯ (p - p2)| / |p2 - p1|.
-    # Expanding out all terms and grouping according to what Quadric expects
-    # gives the following coefficients.
-    kwargs['a'] = dy*dy + dz*dz
-    kwargs['b'] = dx*dx + dz*dz
-    kwargs['c'] = dx*dx + dy*dy
-    kwargs['d'] = -2*dx*dy
-    kwargs['e'] = -2*dy*dz
-    kwargs['f'] = -2*dx*dz
-    kwargs['g'] = 2*(cy*dz - cz*dy)
-    kwargs['h'] = 2*(cz*dx - cx*dz)
-    kwargs['j'] = 2*(cx*dy - cy*dx)
-    kwargs['k'] = cx*cx + cy*cy + cz*cz - (dx*dx + dy*dy + dz*dz)*r*r
-
-    return Quadric(**kwargs)
+cylinder_from_points = Cylinder.from_points
 
 
 def subdivide(surfaces):

openmc/surface.py

@@ -3,13 +3,14 @@ from collections import OrderedDict
 from copy import deepcopy
 from numbers import Real
 from xml.etree import ElementTree as ET
-from warnings import warn
+from warnings import warn, catch_warnings, simplefilter
+import math
 
 import numpy as np
 
 from openmc.checkvalue import check_type, check_value
 from openmc.region import Region, Intersection, Union
-from openmc.mixin import IDManagerMixin
+from openmc.mixin import IDManagerMixin, IDWarning
 
 
 _BOUNDARY_TYPES = ['transmission', 'vacuum', 'reflective', 'periodic', 'white']
@@ -369,10 +370,32 @@ class PlaneMixin(metaclass=ABCMeta):
         return np.array((a, b, c)) / math.sqrt(a*a + b*b + c*c)
 
     def bounding_box(self, side):
+        """Determine an axis-aligned bounding box.
+
+        An axis-aligned bounding box for Plane half-spaces is represented by
+        its lower-left and upper-right coordinates. If the half-space is
+        unbounded in a particular direction, numpy.inf is used to represent
+        infinity.
+
+        Parameters
+        ----------
+        side : {'+', '-'}
+            Indicates the negative or positive half-space
+
+        Returns
+        -------
+        numpy.ndarray
+            Lower-left coordinates of the axis-aligned bounding box for the
+            desired half-space
+        numpy.ndarray
+            Upper-right coordinates of the axis-aligned bounding box for the
+            desired half-space
+
+        """
         # Compute the bounding box based on the normal vector to the plane
         nhat = self._get_normal()
-        lb = np.array([-np.inf, -np.inf, -np.inf])
-        ub = np.array([np.inf, np.inf, np.inf])
+        ll = np.array([-np.inf, -np.inf, -np.inf])
+        ur = np.array([np.inf, np.inf, np.inf])
         # If the plane is axis aligned, find the proper bounding box
         if np.any(np.isclose(np.abs(nhat), 1., rtol=0., atol=self._atol)):
             sign = nhat.sum()
@@ -380,16 +403,16 @@ class PlaneMixin(metaclass=ABCMeta):
             vals = [d/val if round(val) != 0 else np.nan for val in (a, b, c)]
             if side == '-':
                 if sign > 0:
-                    ub = np.array([v if not np.isnan(v) else np.inf for v in vals])
+                    ur = np.array([v if not np.isnan(v) else np.inf for v in vals])
                 else:
-                    lb = np.array([v if ~np.isnan(v) else -np.inf for v in vals])
+                    ll = np.array([v if not np.isnan(v) else -np.inf for v in vals])
             elif side == '+':
                 if sign > 0:
-                    lb = np.array([v if ~np.isnan(v) else -np.inf for v in vals])
+                    ll = np.array([v if not np.isnan(v) else -np.inf for v in vals])
                 else:
-                    ub = np.array([v if ~np.isnan(v) else np.inf for v in vals])
+                    ur = np.array([v if not np.isnan(v) else np.inf for v in vals])
 
-        return (lb, ub)
+        return (ll, ur)
 
     def evaluate(self, point):
         """Evaluate the surface equation at a given point.
@@ -1180,18 +1203,12 @@ class Cylinder(QuadricMixin, Surface):
         """
         # This method overrides Surface.to_xml_element to generate a Quadric
         # since the C++ layer doesn't support Cylinders right now
-        element = ET.Element("surface")
-        element.set("id", str(self._id))
-
-        if len(self._name) > 0:
-            element.set("name", str(self._name))
-
-        element.set("type", 'quadric')
-        if self.boundary_type != 'transmission':
-            element.set("boundary", self.boundary_type)
-        element.set("coeffs", ' '.join([str(c) for c in self._get_base_coeffs()]))
-
-        return element
+        with catch_warnings():
+            simplefilter('ignore', IDWarning)
+            kwargs = {'boundary_type': self.boundary_type, 'name': self.name,
+                      'surface_id': self.id} 
+            quad_rep = Quadric(*self._get_base_coeffs(), **kwargs)
+        return quad_rep.to_xml_element()
 
 
 class XCylinder(QuadricMixin, Surface):
@@ -1881,19 +1898,13 @@ class Cone(QuadricMixin, Surface):
 
         """
         # This method overrides Surface.to_xml_element to generate a Quadric
-        # since the C++ layer doesn't support Cylinders right now
-        element = ET.Element("surface")
-        element.set("id", str(self._id))
-
-        if len(self._name) > 0:
-            element.set("name", str(self._name))
-
-        element.set("type", 'quadric')
-        if self.boundary_type != 'transmission':
-            element.set("boundary", self.boundary_type)
-        element.set("coeffs", ' '.join([str(c) for c in self._get_base_coeffs()]))
-
-        return element
+        # since the C++ layer doesn't support Cones right now
+        with catch_warnings():
+            simplefilter('ignore', IDWarning)
+            kwargs = {'boundary_type': self.boundary_type, 'name': self.name,
+                      'surface_id': self.id} 
+            quad_rep = Quadric(*self._get_base_coeffs(), **kwargs)
+        return quad_rep.to_xml_element()
 
 
 class XCone(QuadricMixin, Surface):

setup.py

@@ -56,14 +56,13 @@ kwargs = {
         'Topic :: Scientific/Engineering'
         'Programming Language :: C++',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.4',
         'Programming Language :: Python :: 3.5',
         'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
     ],
 
     # Dependencies
-    'python_requires': '>=3.4',
+    'python_requires': '>=3.5',
     'install_requires': [
         'numpy>=1.9', 'h5py', 'scipy', 'ipython', 'matplotlib',
         'pandas', 'lxml', 'uncertainties'

tests/unit_tests/test_surface.py

@@ -370,23 +370,26 @@ def test_cylinder_from_points_axis():
     # (x - 3)^2 + (y - 4)^2 = 2^2
     # x^2 + y^2 - 6x - 8y + 21 = 0
     s = openmc.model.cylinder_from_points((3., 4., 0.), (3., 4., 1.), 2.)
-    assert (s.a, s.b, s.c) == pytest.approx((1., 1., 0.))
-    assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.))
-    assert (s.g, s.h, s.j) == pytest.approx((-6., -8., 0.))
-    assert s.k == pytest.approx(21.)
+    a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs()
+    assert (a, b, c) == pytest.approx((1., 1., 0.))
+    assert (d, e, f) == pytest.approx((0., 0., 0.))
+    assert (g, h, j) == pytest.approx((-6., -8., 0.))
+    assert k == pytest.approx(21.)
 
     # (y + 7)^2 + (z - 1)^2 = 3^2
     # y^2 + z^2 + 14y - 2z + 41 = 0
     s = openmc.model.cylinder_from_points((0., -7, 1.), (1., -7., 1.), 3.)
-    assert (s.a, s.b, s.c) == pytest.approx((0., 1., 1.))
-    assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.))
-    assert (s.g, s.h, s.j) == pytest.approx((0., 14., -2.))
-    assert s.k == 41.
+    a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs()
+    assert (a, b, c) == pytest.approx((0., 1., 1.))
+    assert (d, e, f) == pytest.approx((0., 0., 0.))
+    assert (g, h, j) == pytest.approx((0., 14., -2.))
+    assert k == 41.
 
     # (x - 2)^2 + (z - 5)^2 = 4^2
     # x^2 + z^2 - 4x - 10z + 13 = 0
     s = openmc.model.cylinder_from_points((2., 0., 5.), (2., 1., 5.), 4.)
-    assert (s.a, s.b, s.c) == pytest.approx((1., 0., 1.))
-    assert (s.d, s.e, s.f) == pytest.approx((0., 0., 0.))
-    assert (s.g, s.h, s.j) == pytest.approx((-4., 0., -10.))
-    assert s.k == pytest.approx(13.)
+    a, b, c, d, e, f, g, h, j, k = s._get_base_coeffs()
+    assert (a, b, c) == pytest.approx((1., 0., 1.))
+    assert (d, e, f) == pytest.approx((0., 0., 0.))
+    assert (g, h, j) == pytest.approx((-4., 0., -10.))
+    assert k == pytest.approx(13.)