New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Attaching a pyx file in the presence of __init__.py results in wrong module names #12868
Comments
comment:1
I'm not yet sure whether that's really a bug, or just confusing. (If I understand correctly, the presence of an The behaviour should probably also depend on whether some directory of the path to the attached file is in [W.r.t. the component: "User interface" seems a candidate as well.] |
comment:2
Btw., the output of (in your case)
looks different as well, depending on whether an Without one, there's also:
(The attached file is |
comment:3
Looks like distutils were responsible for that behaviour... |
comment:4
In the Cython-generated ...
/* Module declarations from '_tmp_foo_foo_pyx._tmp_foo_foo_pyx_0' */
#define __Pyx_MODULE_NAME "_tmp_foo_foo_pyx._tmp_foo_foo_pyx_0"
int __pyx_module_is_main__tmp_foo_foo_pyx___tmp_foo_foo_pyx_0 = 0;
/* Implementation of '_tmp_foo_foo_pyx._tmp_foo_foo_pyx_0' */
static char __pyx_k_1[] = "File: _tmp_foo_foo_pyx_0.pyx (starting at line 2)";
static char __pyx_k_2[] = "_tmp_foo_foo_pyx._tmp_foo_foo_pyx_0";
static char __pyx_k__bar[] = "bar";
static char __pyx_k____main__[] = "__main__";
static char __pyx_k____test__[] = "__test__";
static PyObject *__pyx_n_s_2;
static PyObject *__pyx_n_s____main__;
static PyObject *__pyx_n_s____test__;
static PyObject *__pyx_n_s__bar;
static PyMethodDef __pyx_methods[] = {
{0, 0, 0, 0}
};
#if PY_MAJOR_VERSION >= 3
static struct PyModuleDef __pyx_moduledef = {
PyModuleDef_HEAD_INIT,
__Pyx_NAMESTR("_tmp_foo_foo_pyx_0"),
__Pyx_DOCSTR(__pyx_k_1), /* m_doc */
-1, /* m_size */
__pyx_methods /* m_methods */,
NULL, /* m_reload */
NULL, /* m_traverse */
NULL, /* m_clear */
NULL /* m_free */
};
#endif
... with an In the temporary build directory, also a symbolic link to
So I'm pretty sure -- if at all -- distutils are to blame for the unexpected behaviour, although it seems quite reasonable to me. |
comment:5
Replying to @nexttime:
Nope, actually Cython is causing this, since the following does not solve your problem (if it really is one): diff --git a/sage/misc/cython.py b/sage/misc/cython.py
--- a/sage/misc/cython.py
+++ b/sage/misc/cython.py
@@ -432,6 +432,11 @@
SAGE_ROOT = os.environ['SAGE_ROOT']
SAGE_LOCAL = SAGE_ROOT + '/local/'
+# Remove unwanted link to an __init__.py file from the current directory,
+# i.e., from the temporary build directory only (#12868):
+if os.path.exists("__init__.py"):
+ os.unlink("__init__.py")
+
extra_link_args = ['-L' + SAGE_LOCAL + '/lib']
extra_compile_args = %s
You can verify that Cython changes the module name depending on whether an $ sage --sh -c 'cython test.pyx' && grep '^#.*__Pyx_MODULE_NAME' test.c` Although it is not yet clear to me how Cython finds the |
comment:6
Replying to @nexttime:
We have a module containing a class bla, but |
comment:7
Replying to @simon-king-jena:
No, I'd rather say it's a "user error"... ;-) Seriously, my above patch didn't work because the link was removed just too late (since I initially thought the problem was caused by distutils, which it is not). Currently changing the patch to remove the (in your opinion) undesirable link to |
comment:8
Replying to @nexttime:
That wouldn't be a solution. Perhaps the user wants (for some reason) to have it in the folder (in particular, it is no user error). I believe that it should simply not matter for attaching files whether or not Attaching a file should be independent from importing, and both should simultaneously be possible. So, unlinking Up to yesterday, I was in the belief that "attaching a pyx-file" means that
Since the temporary directory would not contain |
comment:9
Replying to @simon-king-jena:
In the temporary build folder? Not sure whether there are situations where this is needed. This one works for me: diff --git a/sage/misc/cython.py b/sage/misc/cython.py
--- a/sage/misc/cython.py
+++ b/sage/misc/cython.py
@@ -382,6 +382,10 @@
os.system(cmd)
if os.path.exists("%s/setup.py" % build_dir):
os.unlink("%s/setup.py" % build_dir)
+ if os.path.exists("%s/__init__.py" % build_dir):
+ sys.stderr.write("Note: Ignoring %s/__init__.py ...\n" % abs_base)
+ sys.stderr.flush()
+ os.unlink("%s/__init__.py" % build_dir)
if compile_message:
print "Compiling %s..."%filename No idea whether it breaks anything else. Note that you have to run In case having
Importing in the usual way should still work, since you don't import from the temporary directory, do you?
That's correct. But in addition, symbolic links to any(!) file in the directory containing the attached file are created in the temporary directory (since e.g. you otherwise wouldn't be able to import or include other files in that directory from the attached file). Above, I've only changed that part.
That's what I was saying. (Unless you try to |
comment:10
Replying to @nexttime:
No, in the original folder.
I see! So, you are not unlinking it from the original directory.
Hm. Difficult to tell. If you have files The best solution would be to patch Cython, so that the wrong naming of modules does not occur (in some post above, I think it was shown that the naming is chosen by Cython, not by distutils). |
comment:11
Replying to @nexttime:
Of course there are other, probably smarter (though not necessarily simpler) ways to achieve this, e.g. by modifying |
comment:12
Replying to @simon-king-jena:
I don't think Cython is wrong by what it does, as Consequently, all |
comment:13
Replying to @simon-king-jena:
That should IMHO be solvable by importing / loading the module differently, although I think you run into the same if for example you create links to extension modules and import from these rather than their original location. So I'm not sure if it really is an error. |
comment:14
Replying to @nexttime:
I think that's actually a cleaner (and probably also safer) solution. diff --git a/sage/misc/cython.py b/sage/misc/cython.py
--- a/sage/misc/cython.py
+++ b/sage/misc/cython.py
@@ -382,6 +382,10 @@
os.system(cmd)
if os.path.exists("%s/setup.py" % build_dir):
os.unlink("%s/setup.py" % build_dir)
+ if os.path.exists("%s/__init__.py" % build_dir):
+ sys.stderr.write("Note: Not ignoring %s/__init__.py ...\n" % abs_base)
+ sys.stderr.flush()
+ # os.unlink("%s/__init__.py" % build_dir)
if compile_message:
print "Compiling %s..."%filename
@@ -527,7 +531,13 @@
if os.system(cmd):
raise RuntimeError, "Error making local copy of shared object library for %s"%filename
- return name, build_dir
+ if os.path.exists("%s/__init__.py" % build_dir):
+ # In that case, the module name Cython creates contains the package
+ # name, i.e., the name of the directory the file is contained in.
+ assert not build_dir.endswith(os.path.sep)
+ return "%s.%s" % (base, name), os.path.dirname(build_dir)
+ else:
+ return name, build_dir
Note that this change has the side effect that now
|
comment:15
Just checked: Imports in the attached file also work properly; the imported modules also get the (temporary) package name prepended:
|
comment:16
I'll attach a proper patch after further testing. |
Create a new folder (say, /home/simon/SAGE/work/tests/) and create a file test.pyx with the content
Start sage, attach the file, and you will find something like
Then, create an
__init__.py
file in the same folder. You will findIn fact, even if
__init__.py
is present, the module is still known under the "short" name, but the class doesn't know about it:Please change the component if you think that there is one that fits less bad than "misc"...
Component: misc
Issue created by migration from https://trac.sagemath.org/ticket/12868
The text was updated successfully, but these errors were encountered: