Python package testing: Circular import in a version.py

doronbehar · September 14, 2023, 11:51am

I’m trying to update and enable tests for Python package pyerfa (link to the expression in my fork), and I’m encountering this issue:

__________________ ERROR collecting erfa/tests/test_ufunc.py ___________________
ImportError while importing test module '/build/pyerfa-2.0.0.3/erfa/tests/test_ufunc.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/nix/store/lx8vhp4fxclp494svlfis3sb2g8z4l9h-python3-3.10.12/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
erfa/__init__.py:5: in <module>
    from .version import version as __version__  # noqa
erfa/version.py:25: in <module>
    from . import ufunc
E   ImportError: cannot import name 'ufunc' from partially initialized module 'erfa' (most likely due to a circular import) (/build/pyerfa-2.0.0.3/erfa/__init__.py)

The accused file from upstream is this. I don’t see anything unusual there… Does anyone with good Python eyes can help?

lelgenio · September 14, 2023, 6:24pm

It’s been some time since I last worked with python, but this definitely looks like a circular import:

github.com

liberfa/pyerfa/blob/5f11b2e90de2874601260c04edc806a3a552fa58/erfa/version.py#L25


      
              warnings.warn(
                  f'could not determine {__name__.split(".")[0]} package version; '
                  f'this indicates a broken installation')
              del warnings
          
          
    _version = '0.0.0'
          
          
# Set the version numbers a bit indirectly, so that Sphinx can pick up
          # up the docstrings and list the values.
          try:
              from . import ufunc
          except ImportError as exc:
              # If compiled to use a system liberfa, that library can be too old, and
              # miss functions that are available in newer liberfa. If so, we should
              # bail since nothing will work, but let's try to give a more informative
              # error message.
              try:
                  from ctypes import CDLL, util, c_char_p
                  liberfa = CDLL(util.find_library('erfa'))
                  liberfa.eraVersion.restype = c_char_p
                  erfa_version = liberfa.eraVersion().decode('ascii')

github.com

liberfa/pyerfa/blob/5f11b2e90de2874601260c04edc806a3a552fa58/erfa/init.py#L5


      
          # Licensed under a 3-clause BSD style license - see LICENSE.rst
          
          
# Import version first, as this gives a more useful error message
          # if a system liberfa is too old.
          from .version import version as __version__  # noqa
          from .core import *  # noqa
          from .ufunc import (dt_eraASTROM, dt_eraLDBODY, dt_eraLEAPSECOND,  # noqa
                              dt_pv, dt_sign, dt_type, dt_ymdf, dt_hmsf, dt_dmsf)
          from .helpers import leap_seconds  # noqa

abathur · September 14, 2023, 7:47pm

Looks like maybe the same issue as Problem with importing erfa and astropy or photutils in iPython/Spyder · Issue #92 · liberfa/pyerfa · GitHub?

I’m not sure off the top of my head. I see some template files in the repo (https://github.com/search?q=repo%3Aliberfa%2Fpyerfa%20path%3A**%2F*.templ&type=code) so I guess one thread to pull on is making sure that those are actually getting rendered out as expected and into the expected locations during the build process?

I don’t see any non-standard invocations in the Nix expression, so we might be missing a step if they explicitly run it locally or in CI?

doronbehar · September 15, 2023, 7:30am

Thanks for trying to help @lelgenio ! Are you suggesting that in general, every time a package uses from . import foo or import .foo etc is doing it wrong? I may write an upstream patch that will fix it all… It’s just surprising to me that this is so abundant in this code.

doronbehar · September 15, 2023, 8:06am

It’s the weirdest thing… I replaced every . with erfa in all imports, and it sort of fixed the issue now, but I had also to add rm -rf erfa in the preCheck to make the build work, because it seems the python interpreter didn’t know whether it should import erfa from $out or from $PWD… This scenario seems rather common in Nixpkgs to me - a Python package naming both the python files’ directory the same as the module (the <> in import <>) - hence it’s so weird that I have to rm -rf erfa in preCheck…

EDIT: Now a detailed investigation is available at:

github.com/NixOS/nixpkgs

pytestCheckHook: Switch from `python -m pytest` to simply `pytest`?

opened 09:47AM - 15 Sep 23 UTC

doronbehar

0.kind: bug 6.topic: python

## Summary of issue Many packages in Nixpkgs that use cython extension module…s, demonstrate some issues with running tests via `pytestCheckHook`, credit to @natsukium for helping figuring that out. Generally, the problem is that the extension modules are installed at the `installCheckPhase` at `$out`, and are not `$PWD`. This for instance may cause differences between running `pytest` and running `python -m pytest` - hence the issue title. What's rather peculiar, is the fact that all packages that manage to overcome that use different schemes. The most common of them all is: ```nix preCheck = '' cd $out ''; ``` Below is a list of such packages, and PRs that improved the situation a bit: - [ ] https://github.com/NixOS/nixpkgs/pull/255254 - [ ] https://github.com/NixOS/nixpkgs/pull/255256 - [ ] https://github.com/NixOS/nixpkgs/pull/255260 - [ ] https://github.com/NixOS/nixpkgs/pull/255471 - [ ] https://github.com/NixOS/nixpkgs/pull/255124 - [ ] https://github.com/NixOS/nixpkgs/pull/251923 - [ ] https://github.com/NixOS/nixpkgs/pull/255607 - [ ] https://github.com/NixOS/nixpkgs/pull/263989#discussion_r1375276452 - [ ] `python3.pkgs.orange3` No PR was made yet to workaround the issue. - [ ] https://github.com/NixOS/nixpkgs/pull/263989#pullrequestreview-1702811959 - [ ] https://github.com/NixOS/nixpkgs/pull/228099#discussion_r1413042131 - [ ] `python3.pkgs.matplotlib`, the common workaround doesn't work due to the directory structure of it, and no other workaround has been found yet. See also https://github.com/NixOS/nixpkgs/pull/279317 . Below is the original issue description, opened before further details were investigated. Hence the issues there may be a bit outdated. ## Original issue description ### Describe the bug While trying to enable tests for [pyerfa](https://github.com/liberfa/pyerfa), I noticed that I got circular imports errors as explained upstream [here](https://github.com/liberfa/pyerfa/issues/112#issuecomment-1721212267). I then ran `git grep circular import` and learned that another Python package, [`pyrevolve`](https://github.com/devitocodes/pyrevolve) is experiencing the same issue on NixOS: https://github.com/NixOS/nixpkgs/blob/46688f8eb5cd6f1298d873d4d2b9cf245e09e88e/pkgs/development/python-modules/pyrevolve/default.nix#L36-L46 ### Steps To Reproduce For each package that comes out in a `git grep circular import` search, I'll list what I learned, from most interesting, to less interesting: #### `python3.pkgs.pyrevolve` Try to build it with this patch: ```diff diff --git i/pkgs/development/python-modules/pyrevolve/default.nix w/pkgs/development/python-modules/pyrevolve/default.nix index 754baf91ad38..33df5d177f4d 100644 --- i/pkgs/development/python-modules/pyrevolve/default.nix +++ w/pkgs/development/python-modules/pyrevolve/default.nix @@ -38,7 +38,7 @@ buildPythonPackage rec { # ImportError: cannot import name 'crevolve' from partially initialized module 'pyrevolve' # (most likely due to a circular import) checkPhase = '' - pytest + python -m pytest ''; pythonImportsCheck = [ ``` Which does the same as replacing `pytest` with `pytestCheckHook`, and removing the overriding `checkPhase`. Note how the tests now fail. #### `python3.pkgs.orange3` Seems like a very similar case to what happens in `python3.pkgs.pyrevolve`, but replacing `python -m pytest` with `pytest` doesn't resolve the issue. I also haven't opened an issue upstream because I'm not sure whether this is our fault or theirs. <details><summary>I tested this issue using this patch</summary> ```diff diff --git i/pkgs/development/python-modules/orange3/default.nix w/pkgs/development/python-modules/orange3/default.nix index cff4a603c846..4a9051548655 100644 --- i/pkgs/development/python-modules/orange3/default.nix +++ w/pkgs/development/python-modules/orange3/default.nix @@ -24,6 +24,7 @@ , pyqtgraph , pyqtwebengine , python +, pytest , python-louvain , pythonOlder , pyyaml @@ -104,8 +105,13 @@ let baycomp ]; - # FIXME: ImportError: cannot import name '_variable' from partially initialized module 'Orange.data' (most likely due to a circular import) (/build/source/Orange/data/__init__.py) - doCheck = false; + doCheck = true; + nativeCheckInputs = [ + pytest + ]; + checkPhase = '' + pytest + ''; pythonImportsCheck = [ "Orange" "Orange.data._variable" ]; ``` </details> #### `python3.pkgs.primer3` The circular import issue is mentioned there in a comment, but that comment seems maybe inaccurate to me, and the same testing failure is observed when adding `pytest` to `nativeCheckInputs`, and when running manually `pytest` in `checkPhase`. Comment improvement is in https://github.com/NixOS/nixpkgs/pull/255260 #### `python3.pkgs.xdot` Don't know why was the purpose of the `circular import` comment, maybe it was outdated. Now a fix for tests and update is available in https://github.com/NixOS/nixpkgs/pull/255254 . #### `python3.pkgs.iniconfig` Not exactly this issue, but rather it is a form of https://github.com/NixOS/nixpkgs/issues/63168 . Improvement for the comment near it's `doCheck = false;` is in https://github.com/NixOS/nixpkgs/pull/255256 . ### Expected behavior No difference between `pytest` and `python -m pytest`. ### Additional context Problem was investigated also in https://discourse.nixos.org/t/python-package-testing-circular-import-in-a-version-py/33019/ . ### Notify maintainers @FRidh . Maybe `python3.pkgs.orange3`, and `python3.pkgs.pyrevolve` maintainers @lucasew and @AtilaSaraiva will be also interested. </details>