Skip to content
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

py3: import error #27472

Closed
fchapoton opened this issue Mar 12, 2019 · 16 comments
Closed

py3: import error #27472

fchapoton opened this issue Mar 12, 2019 · 16 comments

Comments

@fchapoton
Copy link
Contributor

This is bug report on python3-build sage.

When attaching a file which loads a pyx file, the reloading fails.

Steps to reproduce:

create a file "test.py" with

from sage.misc.persist import load
load("anyfile.pyx")

def cool(n):
    return -n

and a pyx file with the chosen name.

Then attach the test.py file. This should compile the pyx file.

Then modify a function in the test.py file and save it.

Then one gets an error:

### reloading attached file test.py modified at 16:49:10 ###
  File "<string>", line 1
    from _home_chapoton_anyfile_pyx_0.cpython-36m-x86_64-linux-gnu import *
                                                                                    ^
SyntaxError: invalid syntax

I suspect that maybe the issue is the presence of - in the name of the created python module.

CC: @embray @jdemeyer @kiwifb @tscrim @vinklein

Component: python3

Author: Erik Bray, Frédéric Chapoton

Branch/Commit: d29aa5c

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/27472

@fchapoton fchapoton added this to the sage-8.7 milestone Mar 12, 2019
@embray
Copy link
Contributor

embray commented Mar 12, 2019

comment:1

I haven't looked at the code yet, but apparently it's generating some code (which it perhaps shouldn't be doing in the first place?) which is deriving a Python module name from the filename by dropping the .pyx extension. However on Python 3 this is not good enough for extension modules--you need to drop the full extension module suffix from importlib.machinery.EXTENSION_SUFFIXES[0].

@embray
Copy link
Contributor

embray commented Mar 12, 2019

comment:2

I feel like I've fixed some other issues related to this, but I don't recall if this is one of them.

@embray
Copy link
Contributor

embray commented Mar 12, 2019

comment:3

I have this patch floating around in my old python3 branch which might do the trick. I'm going to try it later (no time right this minute):

diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
index a231502..db1ef3e 100644
--- a/src/sage/misc/sageinspect.py
+++ b/src/sage/misc/sageinspect.py
@@ -122,11 +122,18 @@ import ast
 import inspect
 import functools
 import os
-import tokenize
 import re
+import sys
+import tokenize
 EMBEDDED_MODE = False
 from sage.env import SAGE_SRC

+try:
+    import importlib.machinery as import_machinery
+except ImportError:
+    pass
+
 def loadable_module_extension():
     r"""
     Return the filename extension of loadable modules, including the dot.
@@ -138,11 +145,17 @@ def loadable_module_extension():
         sage: sage.structure.sage_object.__file__.endswith(loadable_module_extension())
         True
     """
-    import sys
-    if sys.platform == 'cygwin':
-        return os.path.extsep+'dll'
+
+    if six.PY2:
+        if sys.platform == 'cygwin':
+            return os.path.extsep + 'dll'
+        else:
+            return os.path.extsep + 'so'
     else:
-        return os.path.extsep+'so'
+        # Return the full platform-specific extension module
+        # extension
+        return import_machinery.EXTENSION_SUFFIXES[0]
+

 def isclassinstance(obj):
     r"""

@fchapoton
Copy link
Contributor Author

Branch: public/ticket/27472

@fchapoton
Copy link
Contributor Author

comment:4

I have made a branch from your diff, but not tested yet.


New commits:

3306554py3: branch to describe correct pyx extension suffix

@fchapoton
Copy link
Contributor Author

Commit: 3306554

@embray
Copy link
Contributor

embray commented Mar 13, 2019

comment:5

Thanks. I will try it out now. I would have yesterday but I was in a rush to get out the door.

@embray
Copy link
Contributor

embray commented Mar 13, 2019

comment:6

I went through your example (is there an existing test that might reproduce this?) and, with this branch, it worked a couple times:

sage: attach('_test.py')
Compiling ./_test2.pyx...
### reloading attached file _test.py modified at 16:34:31 ###
### reloading attached file _test.py modified at 16:34:46 ###

where the "reloading" messages came after me modifying and saving the file. But I tried it a few more times and then (seemingly randomly) got this apparently unrelated problem:

sage: ### detaching file /home/embray/src/sagemath/sage-python3/_test.py because it does not exist (deleted?) ###
sage:

**********************************************************************

Oops, Sage crashed. We do our best to make it stable, but...
...

along with the usual message when the interpreter itself crashes.

The crash reported ends in:

/home/embray/src/sagemath/sage-python3/local/lib/python3.6/site-packages/sage/repl/inputhook.py in sage_inputhook(context=<prompt_toolkit.eventloop.inputhook.InputHookContext object>)
     19 import select
     20 import errno
     21
     22 from IPython import get_ipython
     23 from IPython.terminal.pt_inputhooks import register
     24
     25 import sage.repl.attach
     26
     27
     28 TIMEOUT = 0.25   # seconds
     29
     30
     31 def sage_inputhook(context):
     32     f = context.fileno()
     33     while True:
---> 34         sage.repl.attach.reload_attached_files_if_modified()
        global sage.repl.attach.reload_attached_files_if_modified = <function reload_attached_files_if_modified at 0x7fe495c6b620>
     35         try:
     36             r, w, e = select.select([f], [], [], TIMEOUT)
     37             if f in r:
     38                 return  # IPython signalled us to stop
     39         except select.error as e:
     40             if e[0] != errno.EINTR:
     41                 raise
     42
     43
     44 register('sage', sage_inputhook)
     45
     46
     47 def install():
     48     """
     49     Install the Sage input hook

/home/embray/src/sagemath/sage-python3/local/lib/python3.6/site-packages/sage/repl/attach.py in reload_attached_files_if_modified()
    579     where the automatic reload is triggered. So we have to do it
    580     manually::
    581
    582         sage: shell.run_cell('from sage.repl.attach import reload_attached_files_if_modified')
    583         sage: shell.run_cell('reload_attached_files_if_modified()')
    584         ### reloading attached file tmp_....py modified at ... ###
    585
    586         sage: shell.run_cell('a')
    587         3
    588         sage: shell.run_cell('detach({0})'.format(repr(tmp)))
    589         sage: shell.run_cell('attached_files()')
    590         []
    591         sage: shell.quit()
    592     """
    593     ip = get_ipython()
--> 594     for filename, mtime in modified_file_iterator():
        filename = undefined
        mtime = undefined
        global modified_file_iterator = <function modified_file_iterator at 0x7fe495c6b268>
    595         basename = os.path.basename(filename)
    596         timestr = time.strftime('%T', mtime)
    597         notice = '### reloading attached file {0} modified at {1} ###'.format(basename, timestr)
    598         if ip and ip.pt_cli:
    599             with ip.pt_cli.patch_stdout_context(raw=True):
    600                 print(notice)
    601                 code = load_wrap(filename, attach=True)
    602                 ip.run_cell(code)
    603         elif ip:
    604             print(notice)
    605             code = load_wrap(filename, attach=True)
    606             ip.run_cell(code)
    607         else:
    608             print(notice)
    609             load(filename, globals(), attach=True)

/home/embray/src/sagemath/sage-python3/local/lib/python3.6/site-packages/sage/repl/attach.py in modified_file_iterator()
    521     EXAMPLES::
    522
    523         sage: sage.repl.attach.reset()
    524         sage: t = tmp_filename(ext='.py')
    525         sage: attach(t)
    526         sage: from sage.repl.attach import modified_file_iterator
    527         sage: list(modified_file_iterator())
    528         []
    529         sage: sleep(1)   # filesystem mtime granularity
    530         sage: _ = open(t, 'w').write('1')
    531         sage: list(modified_file_iterator())
    532         [('/.../tmp_....py', time.struct_time(...))]
    533     """
    534     global attached
    535     modified = dict()
--> 536     for filename in attached.keys():
        filename = '/home/embray/src/sagemath/sage-python3/_test.py'
        global attached.keys = <built-in method keys of dict object at 0x7fe494bbd2d0>
    537         old_tm = attached[filename]
    538         if not os.path.exists(filename):
    539             print('### detaching file {0} because it does not exist (deleted?) ###'.format(filename))
    540             detach(filename)
    541             continue
    542         new_tm = os.path.getmtime(filename)
    543         if new_tm > old_tm:
    544             modified[filename] = new_tm
    545
    546     if not modified:
    547         return
    548     time.sleep(0.1)  # sleep 100ms to give the editor time to finish saving
    549
    550     for filename in modified.keys():
    551         old_tm = modified[filename]

RuntimeError: dictionary changed size during iteration

I think this might be due to a race with my editor and/or filesystem--I was editing in vim which writes changes to a swap file, and then when you save copies the swap file over the actual filename of the file.

The actual crash--the RuntimeError: dictionary changed size during iteration--would be specific to Python 3 (since attached.keys() is now a view instead of returning a list), though the original cause of the error is still slightly fishy. It might be best if it retried a couple times before claiming the file has been deleted, in service to editors that work like this.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

2fa44a8Merge branch 'public/ticket/27472' in 8.7.rc0
d29aa5cfixing some .keys() issues in attach.py

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Changed commit from 3306554 to d29aa5c

@fchapoton
Copy link
Contributor Author

comment:8

This seems to fix my %attach problem, and the patchbot is green. Please review my last commit.

@fchapoton
Copy link
Contributor Author

Author: Erik Bray, Frédéric Chapoton

@fchapoton
Copy link
Contributor Author

comment:10

green bot, please review

@kiwifb
Copy link
Member

kiwifb commented Apr 6, 2019

comment:11

LGTM.

@kiwifb
Copy link
Member

kiwifb commented Apr 6, 2019

Reviewer: François Bissey

@vbraun
Copy link
Member

vbraun commented Apr 8, 2019

Changed branch from public/ticket/27472 to d29aa5c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants