Import issues with relative paths and editable install

[ajoubertza]

We've migrated our PyTango project to scikit-build-core - it has been working well, but the latest release has broken something with editable installs.

With 0.5.1, pip install -e . on Linux-based systems didn't work either, but it was a different issue. Specifically, the compiled extension code, _tango.so in our case, had to be copied to back to the source repository folder to avoid a ModuleNotFoundError: No module named 'tango._tango' when importing our library. More details in our gitlab issue. After copying the file, the import and library worked fine. Interestingly, the file copying step isn't required on macOS (tested using mambaforge).

With 0.6.0, we don't get ModuleNotFoundError: No module named 'tango._tango' anymore, Instead, there are many runtime warnings about objects in the extension code being re-registered have appeared, and then a new failure related to a relative import.

>>> import tango
<frozen importlib._bootstrap>:241: RuntimeWarning: to-Python converter for PyCmdDoneEvent already registered; second conversion method ignored.
<frozen importlib._bootstrap>:241: RuntimeWarning: to-Python converter for PyAttrReadEvent already registered; second conversion method ignored.
...
<frozen importlib._bootstrap>:241: RuntimeWarning: to-Python converter for log4tango::Level::LevelLevel already registered; second conversion method ignored.
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/app/tango/__init__.py", line 451, in <module>
    from .attr_data import AttrData
  File "/app/tango/attr_data.py", line 26, in <module>
    from .utils import is_non_str_seq, is_pure_str
  File "/app/tango/utils.py", line 54, in <module>
    from .constants import AlrmValueNotSpec, StatusNotSet, TgLibVers
ModuleNotFoundError: No module named 'tango.attr_data.constants'

In our case, the constants module is imported from the compiled extension module earlier in __init__.py, so the code above worked previously. Changing the imports above slightly, to import constants again fixed that problem, but then there was a new circular import problem in another module.

I guess it is related to changes in #399. Maybe we have something incompatible in our code that needs to change. I can provide the old and new site-packages/_pytango_editable.py files, if necessary.

P.S. Thanks for the great work on this project!

[LecrisUT]

Can you expand on

In our case, the constants module is imported from the compiled extension module earlier in init.py, so the code above worked previously.

If you can run/mock a tree command to understand where are the cmake compiled modules and where are the python native modules. Also the last line in _pytango_editable.py would be helpful (if you can format it before pasting that would be wonderful)

[ajoubertza]

Sure. For the constants import: in tango/__init__.py it is imported before the import from the local module attr_data.py. This code worked fine with scikit-build-core 0.5.1 (and before we moved to scikit-build-core).

from ._tango import (
    ...
    constants,
    ...
)

from .attr_data import AttrData

Click to expand end of installation log

  [69/69] /opt/conda/bin/cmake -E cmake_symlink_library _tango.so.9.5.0.0  _tango.so.9 _tango.so && :

  *** Installing project into wheel...
  -- Install configuration: "Release"
  -- Installing: /tmp/tmpv5i74smf/wheel/platlib/tango/_tango.so.9.5.0.0
  -- Installing: /tmp/tmpv5i74smf/wheel/platlib/tango/_tango.so.9
  -- Installing: /tmp/tmpv5i74smf/wheel/platlib/tango/_tango.so
  *** Making editable...
  *** Created pytango-9.5.0.dev0-cp310-cp310-manylinux_2_36_aarch64.whl...
  Building editable for pytango (pyproject.toml) ... done
  Created wheel for pytango: filename=pytango-9.5.0.dev0-cp310-cp310-manylinux_2_36_aarch64.whl size=5519894 sha256=c16ea946ebffd8669583e5923105617ee4b69fdd99c52a205251ee5ea903921e
  Stored in directory: /tmp/pip-ephem-wheel-cache-1kxd14fg/wheels/9b/2c/d1/15e20a2b97f37ccf65a87ba1049c73a9076d0bf0fbaf814e83
Successfully built pytango
Installing collected packages: pytango
  Attempting uninstall: pytango
    Found existing installation: pytango 9.5.0.dev0
    Uninstalling pytango-9.5.0.dev0:
      Removing file or directory /opt/conda/lib/python3.10/site-packages/__pycache__/_pytango_editable.cpython-310.pyc
      Removing file or directory /opt/conda/lib/python3.10/site-packages/_pytango_editable.pth
      Removing file or directory /opt/conda/lib/python3.10/site-packages/_pytango_editable.py
      Removing file or directory /opt/conda/lib/python3.10/site-packages/pytango-9.5.0.dev0.dist-info/
      Removing file or directory /opt/conda/lib/python3.10/site-packages/tango/
      Successfully uninstalled pytango-9.5.0.dev0
Successfully installed pytango-9.5.0.dev0

Click to expand tree for site-packages and source repo

(base) mambauser@77cb6fcba93a:/opt/conda/lib/python3.10/site-packages/tango$ tree
.
├── _tango.so
├── _tango.so.9
└── _tango.so.9.5.0.0

Source code repo's root is in /app

(base) mambauser@77cb6fcba93a:/app$ tree
.
├── CMakeLists.txt
├── CMakePresets.json
├── CMakeUserPresets.json
├── LICENSE.txt
├── PyTango
│   └── __init__.py
├── README.rst
├── ext
│   ├── api_util.cpp
│   ├── archive_event_info.cpp
│   ...
├── pyproject.toml
├── tango
│   ├── __init__.py
│   ├── api_util.py
│   ├── asyncio.py
│   ├── asyncio_executor.py
│   ├── attr_data.py
│   ...
└── tests
    ├── conftest.py
...

Click to expand for last line in _pytango_editable.py

install({
    'tango': '/app/tango/__init__.py',
    'tango.api_util': '/app/tango/api_util.py',
    'tango.asyncio': '/app/tango/asyncio.py',
    'tango.asyncio_executor': '/app/tango/asyncio_executor.py',
    'tango.attr_data': '/app/tango/attr_data.py',
    'tango.attribute_proxy': '/app/tango/attribute_proxy.py',
    'tango.auto_monitor': '/app/tango/auto_monitor.py',
    'tango.base_types': '/app/tango/base_types.py',
    'tango.callback': '/app/tango/callback.py',
    'tango.client': '/app/tango/client.py',
    'tango.codec': '/app/tango/codec.py',
    'tango.connection': '/app/tango/connection.py',
    'tango.db': '/app/tango/db.py',
    'tango.device_attribute': '/app/tango/device_attribute.py',
    'tango.device_class': '/app/tango/device_class.py',
    'tango.device_data': '/app/tango/device_data.py',
    'tango.device_proxy': '/app/tango/device_proxy.py',
    'tango.device_server': '/app/tango/device_server.py',
    'tango.encoded_attribute': '/app/tango/encoded_attribute.py',
    'tango.exception': '/app/tango/exception.py',
    'tango.futures': '/app/tango/futures.py',
    'tango.futures_executor': '/app/tango/futures_executor.py',
    'tango.gevent': '/app/tango/gevent.py',
    'tango.gevent_executor': '/app/tango/gevent_executor.py',
    'tango.globals': '/app/tango/globals.py',
    'tango.green': '/app/tango/green.py',
    'tango.group': '/app/tango/group.py',
    'tango.group_reply': '/app/tango/group_reply.py',
    'tango.group_reply_list': '/app/tango/group_reply_list.py',
    'tango.log4tango': '/app/tango/log4tango.py',
    'tango.pipe': '/app/tango/pipe.py',
    'tango.pipe_data': '/app/tango/pipe_data.py',
    'tango.pytango_init': '/app/tango/pytango_init.py',
    'tango.pytango_pprint': '/app/tango/pytango_pprint.py',
    'tango.pyutil': '/app/tango/pyutil.py',
    'tango.release': '/app/tango/release.py',
    'tango.server': '/app/tango/server.py',
    'tango.tango_numpy': '/app/tango/tango_numpy.py',
    'tango.tango_object': '/app/tango/tango_object.py',
    'tango.test_context': '/app/tango/test_context.py',
    'tango.test_utils': '/app/tango/test_utils.py',
    'tango.time_val': '/app/tango/time_val.py',
    'tango.utils': '/app/tango/utils.py',
    'tango.databaseds.DataBase': '/app/tango/databaseds/DataBase.xmi',
    'tango.databaseds.DataBaseds': '/app/tango/databaseds/DataBaseds',
    'tango.databaseds': '/app/tango/databaseds/__init__.py',
    'tango.databaseds.create_db': '/app/tango/databaseds/create_db.sql',
    'tango.databaseds.create_db_tables': '/app/tango/databaseds/create_db_tables.sql',
    'tango.databaseds.database': '/app/tango/databaseds/database.py',
    'tango.databaseds.db_errors': '/app/tango/databaseds/db_errors.py',
    'tango.databaseds.mysql2sqlite': '/app/tango/databaseds/mysql2sqlite.sh',
    'tango.databaseds.tango_database': '/app/tango/databaseds/tango_database.db',
    'tango.databaseds.db_access': '/app/tango/databaseds/db_access/__init__.py',
    'tango.databaseds.db_access.sqlite3': '/app/tango/databaseds/db_access/sqlite3.py',
    'PyTango': '/app/PyTango/__init__.py'
},
{
    'tango._tango': 'tango/_tango.so.9.5.0.0'
}, None, False, True,
[
    '-v'
],
[])

[LecrisUT]

Can you give an update of the error you are getting? I don't see where tango.attr_data.constants would be called from in the commit you are currently on. Everything looks fine except that tango.attr_data is a module, not a package, so you shouldn't be able to navigate to any "module" deeper than that. But on the other hand, I do not see anything that would be providing tango.attr_data.constants module, or what would be consuming it as tango.attr_data.constants instead of tango.constants can you clarify that? Also, is constants a module or a class? Maybe there was a typo somewhere?

[ajoubertza]

The constants is a module exported by the extension code (using Boost.Python). The code is in ext/constants.cpp. In tango/__init__.py we import the constants module from the extension module _tango.so.

The tango.attr_data.constants shouldn't be used anywhere, and wasn't. The original error is at line 54 of tango/utils.py, (which is imported from tango/attr_data.py). In utils.py we are trying to import some values from the compiled constants module: from .constants import AlrmValueNotSpec, StatusNotSet, TgLibVers. This should be referencing the constants imported in __init__.py, but now it incorrectly uses the attr_data namespace.

I previously tried removing that line, and using from _tango import constants instead. Then use things like constants.AlrmValueNotSpec. This just caused problems in another place (circular import error).

Any ideas about the new runtime warnings, like <frozen importlib._bootstrap>:241: RuntimeWarning: to-Python converter for PyCmdDoneEvent already registered; second conversion method ignored.? That wasn't happening in version 0.5.1, so maybe it is a clue.

[LecrisUT]

The constants is a module exported by the extension code (using Boost.Python). The code is in ext/constants.cpp. In tango/__init__.py we import the constants module from the extension module _tango.so.

Oh, that complicates stuff. We are using the name of the library to construct the paths for redirection. We do not introspect the library in order to find what modules are defined within it and build the redirection paths. But I also don't know where it should be pointing in this situation, maybe _tango.so?

Can you try the following:

• Change PyImport_AddModule("tango.constants") to PyImport_AddModule("tango._tango.constants"). The tango/__init__.py should be fine as is I think
• If it doesn't work with that change alone, try to add 'tango._tango.constants': 'tango/_tango.so.9.5.0.0' in the _pytango_editable.py

<frozen importlib._bootstrap>:241: RuntimeWarning: to-Python converter for PyCmdDoneEvent already registered; second conversion method ignored.

I think this one is due to running multiple import module commands (probably on the same file). Should be related to the current issue.

[burgholzer]

Maybe it helps: We are also experiencing similar issues over at https://github.com/cda-tum/mqt-core/actions/runs/6615902164/job/17969326394.

__________________________________________________________________________________ ERROR collecting test/python/test_qiskit.py ___________________________________________________________________________________
ImportError while importing test module '/Users/burgholzer/CLionProjects/mqt-core-2/test/python/test_qiskit.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
test/python/test_qiskit.py:12: in <module>
    from mqt.core.operations import CompoundOperation, SymbolicOperation
src/mqt/core/__init__.py:5: in <module>
    from . import operations, symbolic
src/mqt/core/symbolic.py:5: in <module>
    from ._core.symbolic import Expression, Term, Variable
E   ImportError: generic_type: type "Permutation" is already registered!

I can confirm that downgrading to <0.6 as well as non-editable installs resolve the error on our side.
Code is living here: https://github.com/cda-tum/mqt-core
Scikit-build-core config: https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/pyproject.toml#L70-L108
CMake file for the pybind11 extension module: https://github.com/cda-tum/mqt-core/blob/main/src/python/CMakeLists.txt

[henryiii]

Unrelated: pybind11 or nanobind? Pybind11 doesn't set support Stable ABI / Limited API (though it should be possible for CPython 3.12+), so https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/pyproject.toml#L84-L85 shouldn't work with pybind11 on CPython 3.12+ (yet).

[henryiii]

Also, are you coping any files around, like the OP, or is that "out of the box" behavior?

[burgholzer]

Unrelated: pybind11 or nanobind? Pybind11 doesn't set support Stable ABI / Limited API (though it should be possible for CPython 3.12+), so https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/pyproject.toml#L84-L85 shouldn't work with pybind11 on CPython 3.12+ (yet).

We use pybind11 for now.
Maybe I am misinterpreting the output, but https://github.com/cda-tum/mqt-core/actions/runs/6518458808/job/17703904054 seems to suggest that cibuildwheel is producing a abi3 wheel for Python 3.12.
So I figured it is working as intended. Maybe I am wrong though.

Also, are you coping any files around, like the OP, or is that "out of the box" behavior?

I am not copying any files around. The only part where I am setting some kind of path is in the install command here:
https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/src/python/CMakeLists.txt#L65-L66

Might namespace packages create some trouble here?

[henryiii]

Pybind11 isn't compatible with ABI3 yet, at least as far as I know. I'm guessing you aren't defining Py_LIMITED_API before importing Python/pybind11, as so you are making normal wheels, and just calling them ABI3. It will work fine until you try to load it on CPython 3.13, where it will probably segfault or something. (That's a guess, though).

Namespace packages probably do cause trouble. Let's see what we can find, this project looks a bit easier to debug, since it's pybind11 instead of Boost::Python, and no copying is needed.

We might need to gate the change in #399 as an experimental mode.

[LecrisUT]

In both cases there seems to be a sub-module that is trying to be exposed from ._core.symbolic import Expression, Term, Variable. In this case the link to the cmake compiled module seems to have worked, but it is encountering a different issue with the module double-loading. It is curious that these are popping up now. The only significant part we altered was to add submodule_search_locations, which shouldn't have changed how the cmake built libraries are loaded 🤔

[ajoubertza]

@LecrisUT I tried the proposed changes, but no luck.

diff --git a/ext/constants.cpp b/ext/constants.cpp
index 5b56de4..2b184de 100644
--- a/ext/constants.cpp
+++ b/ext/constants.cpp
@@ -91,7 +91,7 @@ static void export_optional_constants(scope& consts_scope)

 void export_constants()
 {
-    object consts_module(handle<>(borrowed(PyImport_AddModule("tango.constants"))));
+    object consts_module(handle<>(borrowed(PyImport_AddModule("tango._tango.constants"))));
     scope().attr("constants") = consts_module;
     scope consts_scope = consts_module;

On its own, no change. Still get ModuleNotFoundError: No module named 'tango.attr_data.constants'.

Changing _pytango_editable.py made no difference (if I did it the right way...):

(base) mambauser@77cb6fcba93a:/$ diff /tmp/_pytango_editable.py.orig /opt/conda/lib/python3.10/site-packages/_pytango_editable.py
255c255,258
<     {"tango._tango": "tango/_tango.so.9.5.0.0"},
---
>     {
>         "tango._tango": "tango/_tango.so.9.5.0.0",
>         "tango._tango.constants": "tango/_tango.so.9.5.0.0",
>     },

[burgholzer]

Pybind11 isn't compatible with ABI3 yet, at least as far as I know. I'm guessing you aren't defining Py_LIMITED_API before importing Python/pybind11, as so you are making normal wheels, and just calling them ABI3. It will work fine until you try to load it on CPython 3.13, where it will probably segfault or something. (That's a guess, though).

I am fairly sure you are right. The only lines regarding the limited ABI we have in the project are
https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/CMakeLists.txt#L35-L38
and
https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/src/python/CMakeLists.txt#L39-L44

I guess we will see once the Python 3.13 betas/release candidates come around.

Namespace packages probably do cause trouble. Let's see what we can find, this project looks a bit easier to debug, since it's pybind11 instead of Boost::Python, and no copying is needed.

We might need to gate the change in #399 as an experimental mode.

Maybe it's also worth to cross-reference wjakob/nanobind_example#30, where some discussion regarding editable installs was going on (although based on nanobind, we found the same considerations apply for pybind11 extensions).

[LecrisUT]

ModuleNotFoundError: No module named 'tango.attr_data.constants'

The source of this issue is weird, I don't get why it is pointing to tango.attr_data. instead of tango.. Can you use absolute path for where constants is called to rule out this particular issue? This seems like a python issue?

@burgholzer for your issue, is the syntax using PYBIND11_MODULE correct here?
https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/src/python/module.cpp#L14C8-L18

Is there an equivalent that defines a package instead of a module? If I am reading the errors correctly it seems the issue is that ._core.symbolic is loaded multiple times (maybe once from ._core and again from ._core.symbolic) with the second time trying to create the module again. Maybe when we did not set submodule_search_locations, it was not able to navigate further down and it was using some cached value, but now that we have "fixed it" these issues are being picked up? These issue seem to be persistent with all python versions as well (not sure about 3.12)

@henryiii Are the API for navigating compiled packages correct in both cases?

[burgholzer]

@burgholzer for your issue, is the syntax using PYBIND11_MODULE correct here? https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/src/python/module.cpp#L14C8-L18

That macro is the entry point for a pybind11 extension; see https://pybind11.readthedocs.io/en/stable/reference.html#c.PYBIND11_MODULE

Is there an equivalent that defines a package instead of a module? If I am reading the errors correctly it seems the issue is that ._core.symbolic is loaded multiple times (maybe once from ._core and again from ._core.symbolic) with the second time trying to create the module again. Maybe when we did not set submodule_search_locations, it was not able to navigate further down and it was using some cached value, but now that we have "fixed it" these issues are being picked up? These issue seem to be persistent with all python versions as well (not sure about 3.12)

I can confirm that the issue is persistent from Python 3.8-3.12 under all major operating systems.
I am not quite sure the .symbolic import has anything to do with this. The error

ImportError: generic_type: type "Permutation" is already registered!

Is completely independent from the symbolic submodule as the permutation submodule is already bound here: https://github.com/cda-tum/mqt-core/blob/a1630ea2ddbeb89ef28e433b106880a9d9f3e343/src/python/module.cpp#L15 and does not at all use or refer to the Permutation class.