Repository: cython/cython
Pull request: Fix failing relative imports in __init__ files.
Fixes #3442

Problem statement:
`from . cimport modulename` tries to search in parent package from `__init__.pxd`.
In `__init__.pxd` in a package, using `from . cimport modulename` attempts to find `modulename.pxd` in the parent directory of the package.

Using `from . import modulename` in basic Python attempts to find `modulename.*` within the package itself, as does using `from . cimport modulename` in all `*.pxd` files *except* for `__init__.pxd` in packages.

Using `from .packagename cimport modulename` in `__init__.pxd` doesn't work around this, because this (correctly) attempts to find `packagename` inside the package itself.

Because of this, I think there should not be much functional code out there that depends on the relative `cimport` being based on the parent directory, meaning that it should be possible to switch it to the package directory, in line with Python behaviour, without breaking compatibility.

---

#### Example structure:
```
cyrel_package/
├── __init__.pxd
├── __init__.py
├── module.pxd
├── module.py
└── subpackage
    ├── __init__.pxd
    ├── __init__.py
    ├── submodule.pxd
    └── submodule.py
```

And in `cyrel_package/subpackage/__init__.pxd`:

```python3
# from . cimport submodule
# Fails, "'cyrel_package/submodule.pxd' not found" — Attempts to import relative to parent package directory instead of current package directory.

# from .subpackage cimport submodule
# Fails, "'cyrel_package/subpackage/subpackage/submodule.pxd' not found" — Attempts to (correctly) import relative to current package directory.

from cyrel_package.subpackage cimport submodule
# Works.
```

It also fails with `from . cimport module` in the topmost `cyrel_package/__init__.pxd`.

Hints from issue discussion:
Thanks for the report. There's probably just a tiny wrong decision taken somewhere, e.g. that `__init__.pxd` is not always considered to be a package in all places or so.

You tried this with the latest master, right? Could you provide a PR that adds this setup to the existing `tests/run/relative_cimport.srctree` test?

Commit message: Fix relative imports in __init__ files by searching __init__ file of ModuleScope

Patch to review:
diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
index 9a3172aaffc..3b39ec2b8b3 100644
--- a/Cython/Compiler/Symtab.py
+++ b/Cython/Compiler/Symtab.py
@@ -5,6 +5,7 @@
 from __future__ import absolute_import
 
 import re
+import os.path
 import copy
 import operator
 
@@ -1351,6 +1352,17 @@ def declare_builtin(self, name, pos):
         entry.qualified_name = self.builtin_scope().qualify_name(name)
         return entry
 
+    def _is_package_scope_or_module(self):
+        # Returns True for all ModuleScopes representing package or scopes representing
+        # modules. Otherwise returns False.
+        # Note: For package pkg_a Cython creates two modules scopes: pkg_a and pkg_a.__init__.
+        # The main purpose of this helper method is to detect pkg_a ModuleScope.
+        path = self.context.search_include_directories(self.qualified_name, suffix='.pyx')
+        if not path:
+            path = self.context.search_include_directories(self.qualified_name, suffix='.py')
+        contains_init = os.path.basename(path) in ('__init__.pyx', '__init__.py') if path else False
+        return self.is_package or not contains_init
+
     def find_module(self, module_name, pos, relative_level=-1):
         # Find a module in the import namespace, interpreting
         # relative imports relative to this module's parent.
@@ -1362,7 +1374,7 @@ def find_module(self, module_name, pos, relative_level=-1):
             # explicit relative cimport
             # error of going beyond top-level is handled in cimport node
             relative_to = self
-            while relative_level > 0 and relative_to:
+            while relative_level > 0 and relative_to and self._is_package_scope_or_module():
                 relative_to = relative_to.parent_module
                 relative_level -= 1
         elif relative_level != 0:
diff --git a/tests/run/relative_cimport.srctree b/tests/run/relative_cimport.srctree
index 2989f9ac50e..29871ff32e6 100644
--- a/tests/run/relative_cimport.srctree
+++ b/tests/run/relative_cimport.srctree
@@ -5,6 +5,7 @@ PYTHON setup.py build_ext --inplace
 PYTHON -c "from pkg.b import test; assert test() == (1, 2)"
 PYTHON -c "from pkg.b_py2 import test; assert test() == (1, 2)"
 PYTHON -c "from pkg.sub.c import test; assert test() == (1, 2)"
+PYTHON -c "from pkg import c"
 
 ######## setup.py ########
 
@@ -19,7 +20,11 @@ setup(
 
 ######## pkg/__init__.py ########
 
-######## pkg/sub/__init__.py ########
+######## pkg/sub/__init__.pyx ########
+
+######## pkg/sub/__init__.pxd ########
+
+from . cimport tdef
 
 ######## pkg/a.pyx ########
 
@@ -53,6 +58,14 @@ def test():
     return (obj.x, obj.y)
 
 
+######## pkg/c.pyx ########
+
+from . cimport sub
+
+cdef sub.tdef.myint i = 5
+assert i == 5
+
+
 ######## pkg/b_py2.pyx ########
 
 # cython: language_level=2


Code review comments:
diff_hunk: @@ -1362,7 +1374,7 @@ def find_module(self, module_name, pos, relative_level=-1):
             # explicit relative cimport
             # error of going beyond top-level is handled in cimport node
             relative_to = self
-            while relative_level > 0 and relative_to:
+            while relative_level > 0 and relative_to and self._is_package_scope_or_module():
path: Cython/Compiler/Symtab.py
text: @user1:
This will probably be reworked, but it seems wasteful to do something as expensive as this as part of a loop, even though it's obvious that the result will never change. Which, BTW, seems problematic. My guess is that you'd want to check parent packages as well, not just the package directory itself.

@author:
Also reworked in next version of PR.

diff_hunk: @@ -1351,6 +1352,17 @@ def declare_builtin(self, name, pos):
         entry.qualified_name = self.builtin_scope().qualify_name(name)
         return entry
 
+    def _is_package_scope_or_module(self):
+        # Returns True for all ModuleScopes representing package or scopes representing
+        # modules. Otherwise returns False.
+        # Note: For package pkg_a Cython creates two modules scopes: pkg_a and pkg_a.__init__.
path: Cython/Compiler/Symtab.py
text: @user1:
I wonder if it's even correct that we create two separate module scopes. And a good thing. It seems to me that the answer might be no.

@author:
Yeas I agree. I fixed in in next version of this PR - see below.