Repository: pyvista/pyiges

Issue:
Request if there is interest to detach pyiges from vtk
Hello,

I'd like to know if You would be interested in decoupling `pyiges` from `vtk` into a pure parsing library?

As there is no other comparable python library for IGES parsing, I think modularity could lead to more people using it, which then could also contribute.

I needed exactly this and already stripped the VTK-part away for me, but if there is interest, I could provide it and help in integrating the pure-parsing code to `pyvista/pyiges`.

**Roadmap**

- [x] increase Test coverage from given example data to 85%, see #30 
- [ ] increase Test coverage with new example data  to 100%, see #31 
- [ ] decouple pure parser as specified in https://github.com/pyvista/pyiges/issues/28#issuecomment-1499524376

Discussion:
mikamove: @banesullivan Dear Bane, this is an implementation of #28 Please feel free to have a look and give me feedback or critique
mikamove: @banesullivan If this being merged, it would make sense to directly release a new version. You might want to change `pyiges/_version.py::version_info` for that. I didn't do so as I'm not familiar with Your versioning practice.
akaszynski: I'm happy with these changes thus far, however, we might consider (as you suggested) skipping tests when not installed with all dependencies. You can do this with [pytest.mark.skipif](https://docs.pytest.org/en/7.3.x/how-to/skipping.html#id1) and then mark all the tests requiring the full install. Then, I'd modify the workflow to first install just the base library, run unit tests, and then reinstall the full library and again run the entire test suite.
mikamove: @akaszynski  (I am not authorized to reply in thread) replying to https://github.com/pyvista/pyiges/pull/33#issuecomment-1575608573

I did that, and more: instead of skipping, I also require that an exception with the suggestion to install `pyiges[full]` is actually raised.

To be nitpicky: The test suite skipping mechanism and the module decision mechansim are both based on the same variable `_IS_FULL_MODULE`, hence the detection itself cannot be tested reliably. E.g. if the mechanism always wrongly detects the module to be the pure variant, no tests would detect that (false positives) although this would be a very critical bug.

Therefore we *could* let You additionally specify this information from the workflow pipeline, e.g.,

`pytest ... --expect-full-module=0`

and add a simple test to check the resulting bool `_IS_FULL_MODULE` against this parameter.

Please let me know in case You want this additional testing mechanism!

akaszynski: Upon further review, I think your approach, although non-standard is probably better in this case given your extensive usage of `assert_full_module_variant`. Let's go with the approach you've implemented.

Only change I'd suggest at this point is to modify the workflow such that the "base" library without extras is installed, tests are run, and then the full library is installed. This will involve two separate installs and two runs of `pytest`. Also, as you suggested, you might consider adding a flag to pytest to ensure that on the second run it expects the full module. 

If you need an example for implementing that, please visit the following permalink for how we did that with `pyvista` and and the `--test_downloads` flag.
https://github.com/pyvista/pyvista/blob/23ebd3851bcf0f0f6a2f3d109719bab7f067d140/tests/conftest.py#L209-L210
mikamove: @akaszynski Thanks for Your feedback, I implemented the pytest option and the two pipelines, I think You won't have to modify the workflow, except You want the two pytest calls to be arranged somehow more symmetrically. Possibly You could also release a new version as suggested in https://github.com/pyvista/pyiges/pull/33#issuecomment-1575546578

Patch:
diff --git a/README.rst b/README.rst
index c64f151..7d3633b 100644
--- a/README.rst
+++ b/README.rst
@@ -21,11 +21,17 @@ following entities:
 
 Installation
 ------------
+
+``pyiges`` is offered in a "full" variant including the conversion features
+and a "pure" parsing module variant.
+The pure variant has no conversion features, no dependencies to ``pyvista,geomdl``,
+and can be installed by removing the ``[full]`` specificator from the following commands.
+
 Install with pip using:
 
 .. code::
 
-   pip install pyiges
+   pip install pyiges[full]
 
 Otherwise, if you want the bleeding edge version, feel free to clone
 this repo and install with:
@@ -34,7 +40,9 @@ this repo and install with:
 
    git clone https://github.com/pyvista/pyiges
    cd pyiges
-   pip install .
+   pip install .[full]
+
+Note that the square brackets might need to be escaped or quoted when using ``zsh``.
 
 
 Usage
diff --git a/docs/index.rst b/docs/index.rst
index da7a775..08b92e2 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -28,11 +28,17 @@ Contents
 
 Installation
 ------------
+
+``pyiges`` is offered in a "full" variant including the conversion features
+and a "pure" parsing module variant.
+The pure variant has no conversion features, no dependencies to ``pyvista,geomdl``,
+and can be installed by removing the ``[full]`` specificator from the following commands.
+
 Install with pip using:
 
 .. code::
 
-   pip install pyiges
+   pip install pyiges[full]
 
 Otherwise, if you want the bleeding edge version, feel free to clone
 this repo and install with:
@@ -41,7 +47,9 @@ this repo and install with:
 
    git clone https://github.com/pyvista/pyiges
    cd pyiges
-   pip install .
+   pip install .[full]
+
+Note that the square brackets might need to be escaped or quoted when using ``zsh``.
 
 
 Usage
diff --git a/pyiges/check_imports.py b/pyiges/check_imports.py
new file mode 100644
index 0000000..45b2f5e
--- /dev/null
+++ b/pyiges/check_imports.py
@@ -0,0 +1,20 @@
+
+try:
+    import pyvista
+    import geomdl
+    from pyvista._vtk import vtkAppendPolyData
+    _IS_FULL_MODULE = True
+except (ModuleNotFoundError, ImportError) as exc:
+    pyvista = None
+    geomdl = None
+    vtkAppendPolyData = None
+    _IS_FULL_MODULE = False
+    _PROBLEM_MSG = 'Import from \'{}\' failed, to support this feature '\
+        'please install pyiges[full]'.format(exc.name)
+
+def assert_full_module_variant(inner_func):
+    def safe_func(*a, **kw):
+        if not _IS_FULL_MODULE:
+            raise Exception(_PROBLEM_MSG)
+        return inner_func(*a, **kw)
+    return safe_func
diff --git a/pyiges/geometry.py b/pyiges/geometry.py
index 4ec801c..9824628 100644
--- a/pyiges/geometry.py
+++ b/pyiges/geometry.py
@@ -1,8 +1,8 @@
 import os
 
-from geomdl import NURBS, BSpline
+from pyiges.check_imports import assert_full_module_variant, geomdl, pyvista as pv
+
 import numpy as np
-import pyvista as pv
 
 from pyiges.entity import Entity
 
@@ -18,8 +18,6 @@ def parse_float(str_value):
     except ValueError:
         return float(str_value.lower().replace("d", "e"))
 
-
-
 class Point(Entity):
     """IGES Point"""
 
@@ -56,6 +54,7 @@ class Point(Entity):
     def __str__(self):
         return self.__repr__()
 
+    @assert_full_module_variant
     def to_vtk(self):
         """Point represented as a ``pyvista.PolyData`` Mesh
 
@@ -91,6 +90,7 @@ class Line(Entity):
         s += "To point {0}, {1}, {2}".format(self._x2, self._y2, self._z2)
         return s
 
+    @assert_full_module_variant
     def to_vtk(self, resolution=1):
         """Line represented as a ``pyvista.PolyData`` Mesh
 
@@ -154,6 +154,7 @@ class Transformation(Entity):
                          [self.r31, self.r32, self.r33, self.t3],
                          [0, 0, 0, 1]])
 
+    @assert_full_module_variant
     def _to_vtk(self):
         """Convert to a vtk transformation matrix"""
         vtkmatrix = pv.vtkmatrix_from_array(self.to_affine())
@@ -227,6 +228,7 @@ class ConicArc(Entity):
         info += 'Scalar coefficient:  %f' % self.f
         return info
 
+    @assert_full_module_variant
     def to_vtk(self):
 
         # a*x**2 + b*x*y + c*y**2 + d*x + e*y + f = 0
@@ -305,7 +307,9 @@ class RationalBSplineCurve(Entity):
             s += "Unit normal: {0} {1} {2}".format(self.XNORM, self.YNORM, self.ZNORM)
         return s
 
+    @assert_full_module_variant
     def to_geomdl(self):
+        from geomdl import NURBS
         curve = NURBS.Curve()
         curve.degree = self.M
         curve.ctrlpts = self.control_points
@@ -313,6 +317,7 @@ class RationalBSplineCurve(Entity):
         curve.knotvector = self.T  # Set knot vector
         return curve
 
+    @assert_full_module_variant
     def to_vtk(self, delta=0.01):
         """Set evaluation delta (controls the number of curve points)
         """
@@ -544,8 +549,10 @@ class RationalBSplineSurface(Entity):
         info += '    Control Points: %d' % len(self._cp)
         return info
 
+    @assert_full_module_variant
     def to_geomdl(self):
         """Return a ``geommdl.BSpline.Surface``"""
+        from geomdl import BSpline
         surf = BSpline.Surface()
 
         # Set degrees
@@ -562,6 +569,7 @@ class RationalBSplineSurface(Entity):
         surf.weights = self._weights
         return surf
 
+    @assert_full_module_variant
     def to_vtk(self, delta=0.025):
         """Return a pyvista.PolyData Mesh
 
@@ -621,6 +629,7 @@ class CircularArc(Entity):
         self.y2 = parse_float(parameters[7])
         self._transform = self.d.get('transform', None)
 
+    @assert_full_module_variant
     def to_vtk(self, resolution=20):
         """Circular arc represented as a ``pyvista.PolyData`` Mesh
 
diff --git a/pyiges/iges.py b/pyiges/iges.py
index 0c7c45c..ec1bff6 100644
--- a/pyiges/iges.py
+++ b/pyiges/iges.py
@@ -1,13 +1,10 @@
 from tqdm import tqdm
+
+from pyiges.check_imports import assert_full_module_variant, pyvista, vtkAppendPolyData
+
 from pyiges.entity import Entity
 from pyiges import geometry
 
-try: 
-    import pyvista 
-    from pyvista._vtk import vtkAppendPolyData
-except ImportError:
-    pass 
-
 class Iges():
     """pyiges.Iges object
 
@@ -43,6 +40,7 @@ class Iges():
          <pyiges.entity.Entity at 0x7f7056069910>]
         """
 
+    @assert_full_module_variant
     def to_vtk(self, lines=True, bsplines=True,
                surfaces=True, points=True, delta=0.025, merge=True,
                progress=tqdm):
diff --git a/setup.py b/setup.py
index ad3e03c..1acaa3e 100644
--- a/setup.py
+++ b/setup.py
@@ -36,6 +36,9 @@ setup(
     ],
     description='Pythonic IGES reader',
     url='https://github.com/pyvista/pyiges',
-    install_requires=['tqdm', 'geomdl', 'pyvista>=0.28.0'],
+    install_requires=['tqdm', 'numpy'],
+    extras_require={
+        'full' : ['geomdl', 'pyvista>=0.28.0']
+    },
     package_data={'pyiges.examples': ['impeller.igs', 'sample.igs']}
 )


Test patch:
diff --git a/.github/workflows/testing-and-deployment.yml b/.github/workflows/testing-and-deployment.yml
index 8e1ebe4..6cfbd0b 100644
--- a/.github/workflows/testing-and-deployment.yml
+++ b/.github/workflows/testing-and-deployment.yml
@@ -46,6 +46,8 @@ jobs:
           pip install -r requirements_test.txt --disable-pip-version-check
           cd tests
           pytest -v --cov pyiges --cov-report xml
+          pip install -e ..[full] --disable-pip-version-check --find-links=dist
+          pytest -v --cov pyiges --cov-report xml --expect-full-module
 
       - name: Upload to PyPi
         if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags')
diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 0000000..e071efa
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,3 @@
+
+def pytest_addoption(parser):
+    parser.addoption("--expect-full-module", action='store_true', default=False)
diff --git a/tests/test_pyiges.py b/tests/test_pyiges.py
index d1e384d..be56809 100644
--- a/tests/test_pyiges.py
+++ b/tests/test_pyiges.py
@@ -1,3 +1,4 @@
+import functools
 import numpy as np
 import os
 import pytest
@@ -7,6 +8,20 @@ from pyiges import examples
 
 DIR_TESTS_REFERENCE_DATA = os.path.join(os.path.dirname(__file__), 'reference_data')
 
+def adjust_depending_on_package_variant(test_func):
+    ''' If the full module is present, run test as usual,
+    otherwise assert that the error is triggered as intended.
+    '''
+    @functools.wraps(test_func)
+    def generalized_test_func(*a, **kw):
+        if pyiges.check_imports._IS_FULL_MODULE:
+            return test_func(*a, **kw)
+        else:
+            with pytest.raises(Exception, match=r'install pyiges\[full\]'):
+                return test_func(*a, **kw)
+    return generalized_test_func
+
+
 @pytest.fixture(scope='module')
 def sample():
     return pyiges.read(examples.sample)
@@ -88,17 +103,20 @@ def test_parse_completeness2(sample):
 
 
 @pytest.mark.parametrize('merge', [False, True])
+@adjust_depending_on_package_variant
 def test_iges_to_vtk1(impeller, merge):
     items = impeller.to_vtk(delta=0.5, merge=merge)
     assert items.bounds == pytest.approx((-50.516562808,  49.445007453, -49.510005208,
                                           50.494748866, -997.9630126953125, 1011.0750122070312))
 
 @pytest.mark.parametrize('merge', [False, True])
+@adjust_depending_on_package_variant
 def test_iges_to_vtk2(sample, merge):
     items = sample.to_vtk(merge=merge)
     assert items.bounds == pytest.approx((0.0, 1.0, 0.0, 1.0, 0.0, 0.0))
 
 
+@adjust_depending_on_package_variant
 def test_surfaces_vtk(surf):
     mesh = surf.to_vtk(delta=0.1)
 
@@ -116,6 +134,29 @@ def test_surfaces_vtk(surf):
                                          -9.363301218200274, -45.772995131000016, -8.876323512))
 
 
+@adjust_depending_on_package_variant
+def test_surfaces_to_geomdl(surf):
+    gsurf = surf.to_geomdl()
+    assert gsurf.data['control_points'] == pytest.approx((
+        [-26.902905334, -16.51153913, -8.876323512],
+        [-25.851820351, -15.866440371, -21.167794775],
+        [-25.995725563, -15.954761558, -33.519826531],
+        [-27.332763631, -16.775362758, -45.772995131],
+        [-28.232974775, -14.344404257, -8.876323512],
+        [-27.129924549, -13.783974529, -21.167794775],
+        [-27.28094438, -13.860703585, -33.519826531],
+        [-28.684085103, -14.573601108, -45.772995131],
+        [-29.292803148, -12.033057876, -8.876323512],
+        [-28.148345882, -11.562931461, -21.167794775],
+        [-28.305034796, -11.62729699, -33.519826531],
+        [-29.760847559, -12.225323719, -45.772995131],
+        [-30.067010387, -9.611041892, -8.876323512],
+        [-28.892305175, -9.235542604, -21.167794775],
+        [-29.05313537, -9.286952628, -33.519826531],
+        [-30.547425187, -9.764608432, -45.772995131],
+    ))
+
+
 def test_surfaces_parse(surf):
     assert surf.parameters == []
     assert surf.sequence_number == 3
@@ -230,6 +271,7 @@ def test_bsplines_parse(curve):
     assert str(curve)
 
 
+@adjust_depending_on_package_variant
 def test_bsplines_vtk(curve):
     mesh = curve.to_vtk()
 
@@ -246,7 +288,28 @@ def test_bsplines_vtk(curve):
     assert mesh.bounds == pytest.approx((0.531027642, 0.559345007,
                                          0.006617509, 0.127235606, 0., 0.))
 
+@adjust_depending_on_package_variant
+def test_bsplines_to_geomdl(curve):
+    gcurve = curve.to_geomdl()
+    assert gcurve.data['control_points'] == pytest.approx((
+        [0.531027642, 0.127235606, 0.0, 1.0],
+        [0.53482791, 0.099559574, 0.0, 1.0],
+        [0.537603234, 0.077944022, 0.0, 1.0],
+        [0.539206635, 0.06545597, 0.0, 1.0],
+        [0.540802737, 0.052996393, 0.0, 1.0],
+        [0.542675511, 0.038377045, 0.0, 1.0],
+        [0.544208805, 0.028733039, 0.0, 1.0],
+        [0.546564848, 0.013914155, 0.0, 1.0],
+        [0.548603824, 0.009169564, 0.0, 1.0],
+        [0.551376925, 0.002716702, 0.0, 1.0],
+        [0.554223526, 0.012546793, 0.0, 1.0],
+        [0.556570985, 0.020653212, 0.0, 1.0],
+        [0.557884113, 0.035401282, 0.0, 1.0],
+        [0.55898716, 0.047789894, 0.0, 1.0],
+        [0.559345007, 0.06395869, 0.0, 1.0],
+    ))
 
+@adjust_depending_on_package_variant
 def test_points_vtk(sample):
     points = sample.points(as_vtk=True, merge=True) # pv.PolyData
 
@@ -331,6 +394,7 @@ def test_point_parse(point):
     assert str(point)
 
 
+@adjust_depending_on_package_variant
 def test_point_vtk(point):
     point = point.to_vtk() # pv.PolyData
 
@@ -380,6 +444,7 @@ def test_circular_arcs_parse(carc):
     assert str(carc)
 
 
+@adjust_depending_on_package_variant
 def test_circular_arcs_vtk(carc):
     polydata = carc.to_vtk()
 
@@ -479,6 +544,7 @@ def test_line_parse(line):
     assert str(line)
 
 
+@adjust_depending_on_package_variant
 def test_line_vtk(line):
     polydata = line.to_vtk()
     assert polydata.n_arrays == 2
@@ -533,12 +599,12 @@ def test_transformation_parse(trafo):
     assert repr(trafo)
     assert str(trafo)
 
+@adjust_depending_on_package_variant
 def test_transformation_vtk(trafo):
     trafo = trafo._to_vtk()
     m = trafo.GetMatrix()
     assert m.GetElement(2,3) == pytest.approx(5.67397368511119)
 
-
 def test_example_with_invalid_conic_arc_and_form1_global_line():
     # For this file, the conic arc cannot be parsed.
     # This is either because of a wrong format of the file or a bug in the parsing.
@@ -548,7 +614,7 @@ def test_example_with_invalid_conic_arc_and_form1_global_line():
     assert len(iges.bsplines()) == 1
     assert len(iges.circular_arcs()) == 1
 
-
+@adjust_depending_on_package_variant
 def test_to_vtk(impeller):
     lines = impeller.to_vtk(lines=True, bsplines=False, surfaces=False)
     assert lines.n_points
@@ -575,3 +641,7 @@ def test_parse_separators_from_first_global_line(line, expected_separators):
     else:
         separators = pyiges.Iges._parse_separators_from_first_global_line(line)
         assert separators == expected_separators
+
+def test_package_variant_detection(request):
+    is_full_module_expected = request.config.getoption("--expect-full-module")
+    assert pyiges.check_imports._IS_FULL_MODULE == is_full_module_expected