Python docstrings tests: don't test for CellmlElementType
Closed this issue · 5 comments
I am very confused since it all seems to be working fine on our CI machines, but whenever I test the Python docstrings (using ctest -V -R python_test_docstrings
), I get the following on my MacBook Pro using Python 3.12.4:
Test command: /opt/homebrew/Frameworks/Python.framework/Versions/3.12/bin/python3.12 "test_docstrings.py"
Working Directory: /Users/Alan/libCellML/build/tests/bindings/python
Environment variables:
PYTHONPATH=/Users/Alan/libCellML/build/src/bindings/python
Test timeout computed to be: 10000000
E
======================================================================
ERROR: test_docstrings (__main__.DocstringTestCase.test_docstrings)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/Alan/Work/Programming/libCellML/build/tests/bindings/python/test_docstrings.py", line 31, in test_docstrings
raise Exception(
Exception: Missing (1) docstrings, for: libcellml.CellmlElementType
----------------------------------------------------------------------
Ran 1 test in 0.175s
FAILED (errors=1)
1/1 Test #python_test_docstrings ...........***Failed 0.22 sec
In other words, I get told that there is no docstring for libcellml.CellmlElementType
and indeed I can't see one anywhere. However, if I add a docstring for libcellml::CellmlElementType
in src/bindings/interface/enums.i
, e.g.
--- a/src/bindings/interface/enums.i
+++ b/src/bindings/interface/enums.i
@@ -4,6 +4,9 @@
%include <std_string.i>
+%feature("docstring") libcellml::CellmlElementType
+"Get the :class:`CellmlElementType` of a CellML element.";
+
%feature("docstring") libcellml::cellmlElementTypeAsString
"Convert a CellmlElementType enumeration value into its string form.";
then I still get that error message. @hsorby, any idea of what is going on here?
The only way for me to get rid of that error message is to NOT test whether there is a docstring for libcellml::CellmlElementType
, i.e.
--- a/tests/bindings/python/test_docstrings.py
+++ b/tests/bindings/python/test_docstrings.py
@@ -13,7 +13,7 @@ class DocstringTestCase(unittest.TestCase):
# Scan for missing or empty docstrings
def scan(root, missing, prefix=''):
prefix += root.__name__
- if not root.__doc__:
+ if not root.__doc__ and root.__name__ != 'CellmlElementType':
missing.append(prefix)
prefix += '.'
# Scan children, using dict instead of dir to avoid inherited
The CI machines seem to be using Python 3.7, 3.8, or 3.9 - is this a 3.12 specific issue?
Yup:
- 3.8:
OK
- 3.9:
OK
- 3.10:
OK
- 3.11:
NOT OK
- 3.12:
NOT OK
As a solution here, I would like us to switch to using SWIG to take the Doxygen comments from the C++ and applying it to the Python bindings. This does have some disadvantages but I think the Doxygen documentation is more informative than the current Python bindings documentation. I haven't tested this idea with Python 3.11, or 3.12 to see if it would resolve this particular issue though.
I can say from my experience that SWIG is great at what it does, but it also means that you need separate binaries for every version of Python on every platform, which means at least 12 if not 16 binaries for every release. It's certainly doable, but but it is a little much. If you can manage to put something together with ctypes, you'd only need one binary per platform, which would work for all versions of Python.
ctypes
That's all good (although we are still facing the same issue: lack of resources to do nice-to-have things), but what about now? What should we do about this issue?
I can say from my experience that SWIG is great at what it does, but it also means that you need separate binaries for every version of Python on every platform, which means at least 12 if not 16 binaries for every release. It's certainly doable, but but it is a little much. If you can manage to put something together with ctypes, you'd only need one binary per platform, which would work for all versions of Python.
I have never used ctypes, but when it comes to building Python packages, we use cibuildwheel
, so it's relatively simple, although I do appreciate your point.
When it comes to SWIG, I do find it a bit of an overkill for libCellML since we "only" provide Python and JavaScript bindings. So, we only use SWIG for our Python bindings (JavaScript bindings are generated using Emscripten). Personally, I wish we would be using pybind11
(or even nanobind
).