Opened 2 years ago

Closed 2 years ago

#27472 closed defect (fixed)

py3: import error

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: embray, jdemeyer, fbissey, tscrim, vklein Merged in:
Authors: Erik Bray, Frédéric Chapoton Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: d29aa5c (Commits, GitHub, GitLab) Commit: d29aa5c9b9f90de35c2e0f90144c37ae7b865e9c
Dependencies: Stopgaps:

Status badges

Description

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.

Change History (12)

comment:1 Changed 2 years ago by embray

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].

comment:2 Changed 2 years ago by embray

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

comment:3 Changed 2 years ago by embray

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):

  • src/sage/misc/sageinspect.py

    diff --git a/src/sage/misc/sageinspect.py b/src/sage/misc/sageinspect.py
    index a231502..db1ef3e 100644
    a b import ast (this hunk was shorter than expected) 
    122122import inspect
    123123import functools
    124124import os
    125 import tokenize
    126125import re
     126import sys
     127import tokenize
    127128EMBEDDED_MODE = False
    128129from sage.env import SAGE_SRC
    129130
     131try:
     132    import importlib.machinery as import_machinery
     133except ImportError:
     134    pass
     135
    130136def loadable_module_extension():
    131137    r"""
    132138    Return the filename extension of loadable modules, including the dot.
    def loadable_module_extension(): 
    138145        sage: sage.structure.sage_object.__file__.endswith(loadable_module_extension())
    139146        True
    140147    """
    141     import sys
    142     if sys.platform == 'cygwin':
    143         return os.path.extsep+'dll'
     148
     149    if six.PY2:
     150        if sys.platform == 'cygwin':
     151            return os.path.extsep + 'dll'
     152        else:
     153            return os.path.extsep + 'so'
    144154    else:
    145         return os.path.extsep+'so'
     155        # Return the full platform-specific extension module
     156        # extension
     157        return import_machinery.EXTENSION_SUFFIXES[0]
     158
    146159
    147160def isclassinstance(obj):
    148161    r"""

comment:4 Changed 2 years ago by chapoton

  • Branch set to public/ticket/27472
  • Commit set to 33065548ca6e14209e73b03d31defcfdc51cc659

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


New commits:

3306554py3: branch to describe correct pyx extension suffix

comment:5 Changed 2 years ago by embray

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

comment:6 Changed 2 years ago by embray

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.

comment:7 Changed 2 years ago by git

  • Commit changed from 33065548ca6e14209e73b03d31defcfdc51cc659 to d29aa5c9b9f90de35c2e0f90144c37ae7b865e9c

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

comment:8 Changed 2 years ago by chapoton

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

comment:9 Changed 2 years ago by chapoton

  • Authors set to Erik Bray, Frédéric Chapoton
  • Milestone changed from sage-8.7 to sage-8.8
  • Status changed from new to needs_review

comment:10 Changed 2 years ago by chapoton

  • Cc fbissey tscrim vklein added

green bot, please review

comment:11 Changed 2 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

LGTM.

comment:12 Changed 2 years ago by vbraun

  • Branch changed from public/ticket/27472 to d29aa5c9b9f90de35c2e0f90144c37ae7b865e9c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.