python / cpython

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

IDLE: Add completions for un-imported modules #62966

Open 74f3ea33-270d-42b8-91fe-4b0c7b026031 opened 11 years ago

74f3ea33-270d-42b8-91fe-4b0c7b026031 commented 11 years ago
BPO 18766
Nosy @terryjreedy, @taleinat, @rovitotv
Files
  • autocomplete_unimported_1.patch
  • 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 = None created_at = labels = ['expert-IDLE', 'type-feature'] title = "IDLE: Autocomplete in editor doesn't work for un-imported modules" updated_at = user = 'https://bugs.python.org/philwebster' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'none' closed = False closed_date = None closer = None components = ['IDLE'] creation = creator = 'philwebster' dependencies = [] files = ['31332'] hgrepos = [] issue_num = 18766 keywords = ['patch'] message_count = 9.0 messages = ['195446', '224644', '224663', '226536', '226559', '271213', '271215', '349384', '349433'] nosy_count = 5.0 nosy_names = ['terry.reedy', 'taleinat', 'Todd.Rovito', 'philwebster', 'Saimadhav.Heblikar'] pr_nums = [] priority = 'normal' resolution = None stage = 'test needed' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue18766' versions = ['Python 3.6'] ```

    74f3ea33-270d-42b8-91fe-4b0c7b026031 commented 11 years ago

    If you open a new editor window in IDLE, some modules will autocomplete (re, os, other common modules used by IDLE) and others will not (textwrap, datetime, ...). The current solution is to run your module or type the imports into the shell window to get completions working, which is not very intuitive. I created a patch that checks the namespace and attempts to import the module automatically if it is not found.

    83d2e70e-e599-4a04-b820-3814bbdb9bef commented 10 years ago

    The patch is simple but I don't know enough about IDLE to comment technically, what do you think Terry?

    terryjreedy commented 10 years ago

    A least a manual (human) test is needed: a description of steps that do not work now and do work with a patch.

    Currently, get_entity() is only called in one place in fetch_completions; it is not used anywhere else in idlelib. (Call_tips defines another getentity().) Its body should be moved inline at the call site. A variation that differs in the second line namespace = \_main.__dict.copy() namespace.update(main.__builtins.__dict) is already in-lined in the if-branch above.

    We are trying to eliminate bare excepts: in idlelib -- #59518 -- not expand them or their use. Try blocks should only include the statement or statements that could raise the exception or exceptions caught. So the import, once inlined, should be inside try: except ImportError: return... .

    I do not know what the existing try: except: is meant to catch, but it includes too much, as does the one that follows. That, however, should be a separate patch.

    + namespace = sys.modules.copy() + namespace.update(main.__dict__) seems unnecessary. I think the following should work. namespace[name] = sys.modules[name] A testcase would make sure either way.

    22200024-de1a-4081-ad85-2ac04e6b54d2 commented 10 years ago

    Hi Terry,

    Would it be better if the current inlined code is moved into get_entity? It will also make it easier to test. The try:... except ImportError block be present within the get_entity method itself? Otherwise, I will make a patch on what you said above.

    terryjreedy commented 10 years ago

    This issue is about name completion and not path completion. I have re-reviewed the patch, current code, current behavior, and the fetch_completions doctstring that more or less explains why name completion cannot reliably work given how Idle now works.

    I would add to the docstring that even after running the editor file and before running anything else, there can still be problems. If one adds 'from mod import SomeClass' and types 'SomeClass.', there will be no help. This patch will not help either. Getting completions for 'sc.', where 'sc' is an instance of SomeClass, without running code, is an even harder problem. There is, in a sense, no substitute for running the code up to the point where name completion is wanted. Perhaps an explicit Show Completions(Name completion seems fairly common; I wonder how other editors handle it.)

    Idle once ran user code in the idle process, which meant that any module ever imported by running any editor buffer would still be in sys.modules. Thus looking for modules in the Idle process sys.modules was more or less guaranteed to work, and would substitute for the user __main__ module (within the Idle process) being cleared by running another editor buffer.

    Idle now, without the -N option, runs edited files in a *new* user processes. The problem for name completion (and calltips, and shell interaction with the old process) is that Idle discards the previous user process. So running file B discards the information needed for name completion with file A. A possible solution to this is to keep a user process for each file and not discard information we may want.

    I thought of this idea about a month ago because I am sometimes annoyed when running any file discards an interactive shell process I am not really done with. After running a file, I sometimes retype or rerun code to recreate the bidings I had before running an editor file. This change would also help this issue and a similar one with calltips. I plan to make this proposal the subject of another issue.

    If I open Idle with Shell and open a new editor window, then, as Phil noted, typing 're.' and waiting brings up a list of re attributes, whereas 'datetime.' does not. A year ago, 'textwrap.' did not, whereas now it does, thus illustratig the arbitrary nature of current behavior.

    Phil's proposal is to make 'datetime.' act like 're.' by importing datetime. There are two things I do not like about this. First, if there is no 'import re' in the file, then putting, say, 're.DOTALL' into the file is a bug in that it will be a NameError when the file is run. I might rather make 'import re' work like 'import datetime', that is fail, by not using sys.module as a backup. Second, the import is done in a process that does not belong to the editor, adding a name binding where is does not belong, and will have to be done again when the file is run.

    Phil notes that the current workaround it to do the import in the shell (which, as noted, does it in the wrong place) or run the file (which, as noted, deleted the current process). If there were a separate process for each editor, then either running the process or doing the import would not have the problems listed above. If we did do an auto import, it would be under the presumption that it would be imported into that particular process in a later run. We could also think about doing something else, like checking whether 'import xxx' is in the file and either inserting it or at least warning the user to do so.

    I would add to the docstring that even after running the editor file and before running anything else, there can still be problems. If one adds 'from mod import SomeClass' and types 'SomeClass.', there will be no help. This patch will not help either. Getting completions for 'sc.', where 'sc' is an instance of SomeClass, without running code, is an even harder problem. There is, in a sense, no substitute for running the code up to the point where name completion is wanted. Perhaps an explicit Show Completions or Cntl-Space should do so. This would be more feasible with a separate process so it did not have the side effect of killing the existing user process.

    Name completion seems fairly common; I wonder how other editors handle it.

    --- In my previous post, I suggested that get_entity does too much by copying to namespaces into one, looking up one name, and discarding the new combined namespace. Looking more, it also occurs to me that the eval is not needed, as eval(name, namespace) is the same as getattr(name, namespace). I believe the following would do what the current get_entity does. it now does.

    def get_entity(name):  # should be a standalone utility function
        try:
            ob = __main__.__dict__[name]
        except KeyError:
            try:
                ob = sys.modules[name]
            except KeyError:
               raise NameError('cannot find %s' % name) from None
        return ob

    With this version, the patch would amount to adding try: import name as ob except ImportError

    before giving up and raising.

    I agree that keeping get_entiry separate (as a function, since self is not used) might make testing easier. The same might be reason to pull more code into separate, more easily tested, units. So lets keep it separate for now.

    The real issue is whether the idea of the patch is a good one or not for how Idle currently works.

    terryjreedy commented 8 years ago

    I opened #71796 to keep track of multiple completion issues. Some additions to what I said above.

    1. I would like to move the actual fetch code to run.py, since it is normally executed in the user process. See bpo-27534, but also below.

    2. Idle currently differentiates between completion invocation by \<tab> or '.' and by \<\<force-completions>>, which defaults to Cntl-space. I believe a main difference is that it will only call functions in the object expression, as in "f().", in the latter case. I believe the distinction operates by calling for completions or not. (I need to check.)

    That distinction can be used here by only importing with \<\<force-completion>>, and then it should. With that limitation, I think I am now in favor of adding this. Importing a module is roughly equivalent to calling a function. Either can do anything. But the distinction has be known within the fetch code. This could be done with a wrapper function.

    terryjreedy commented 8 years ago

    The concern about adding bugs could be alleviated by checking whether the name is in an import statement. But it still might not be in the proper form.

    I am trying to reduce the run.py import to those actually needed in run.py. I already reduced sys.modules by 37. The side effect is that fewer modules will autocomplete before running the code in the editor. Hence, there is more need for this change.

    taleinat commented 5 years ago

    I propose closing this as "wontfix", since this goes strongly against how completions currently work in IDLE (based on the namespace of the shell), and I don't think we're considering changing that.

    terryjreedy commented 5 years ago

    The import-on-demand would be executed in the Shell's execution process namespace, so it works with the current implementation. I said above that this puts names 'where they don't belong', but I feel a little differently now. A variant idea might be to execute all imports as entered in the editor in the shell.

    A pie-in-the-sky idea is to give each editor its own execution process and execute each line as entered, or something like that. But I have put off researching what other editors do until we get what we have working better.