python / cpython

The Python programming language
https://www.python.org
Other
63.17k stars 30.25k forks source link

improve linecache: reuse tokenize.detect_encoding() and io.open() #48266

Closed vstinner closed 15 years ago

vstinner commented 16 years ago
BPO 4016
Nosy @amauryfa, @vstinner, @benjaminp
Files
  • linecache_refactor-2.patch: Reuse existing code in linecache.updatecache()
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = created_at = labels = ['library'] title = 'improve linecache: reuse tokenize.detect_encoding() and io.open()' updated_at = user = 'https://github.com/vstinner' ``` bugs.python.org fields: ```python activity = actor = 'benjamin.peterson' assignee = 'none' closed = True closed_date = closer = 'benjamin.peterson' components = ['Library (Lib)'] creation = creator = 'vstinner' dependencies = [] files = ['12333'] hgrepos = [] issue_num = 4016 keywords = ['patch'] message_count = 11.0 messages = ['74165', '74206', '74218', '77643', '77653', '77659', '77684', '78093', '83839', '83840', '84118'] nosy_count = 3.0 nosy_names = ['amaury.forgeotdarc', 'vstinner', 'benjamin.peterson'] pr_nums = [] priority = 'normal' resolution = 'accepted' stage = 'patch review' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue4016' versions = ['Python 3.1'] ```

    vstinner commented 16 years ago

    linecache uses it own code to detect a Python script encoding whereas a function tokenize.detect_encoding() already exists. It does also convert bytes => unicode conversion of the file lines whereas open() already supports this feature.

    vstinner commented 16 years ago

    I wrote a different (and better) patch for tokenize module: moved to the bpo-4021.

    amauryfa commented 16 years ago

    This patch looks good.

    vstinner commented 15 years ago

    "This patch looks good." ok and then?

    benjaminp commented 15 years ago

    Applied in r67713.

    benjaminp commented 15 years ago

    I had to revert this because tokenize imports itertools. This is a problem when building (setup.py) because itertools doesn't exist, yet. I also think we should consider hard adding more modules that are loaded at startup time to py3k already huge list.

    vstinner commented 15 years ago

    It was easy to remove the itertools dependency: only itertools.chain() was really used and a simple "for ...: yield" is enough to get the same behaviour (test_tokenize.py runs fine). With the new version of the patch, the bootstrap works correctly.

    vstinner commented 15 years ago

    I also think we should consider hard adding more modules that are loaded at startup time to py3k already huge list.

    My patch uses tokenize modules in setup.py bootstrap. But it doesn't affect Python classic usage "python" or "python myscript.py".

    vstinner commented 15 years ago

    @benjamin.peterson: The second version of my patch works correctly with the bootstraping.

    I also think we should consider hard adding more modules that are loaded at startup time to py3k already huge list.

    linecache is not loaded at startup time in py3k! I see that linecache is loaded by the warnings module, but the warnings module (Lib/warnings.py) is not loaded at startup. It was maybe the case with Python 2.x or older version of Python 3.x?

    With my patch, loading linecache loads 2 extra modules: tokenize and token. It only impacts code using directly linecache or the warnings module.

    vstinner commented 15 years ago

    Oh, I see that setup.py uses warnings and so linecache is loaded by setup.py.

    benjaminp commented 15 years ago

    Applied in r70587.