Repository navigation
IDLE sys.path does not contain Current Working Directory #57715
Description
| BPO | 13506 |
|---|---|
| Nosy | @terryjreedy, @kbkaiser, @serwy |
| Files |
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:
assignee = 'https://github.com/terryjreedy'
closed_at = <Date 2012-01-31.08:06:37.487>
created_at = <Date 2011-11-30.02:51:04.496>
labels = ['expert-IDLE', 'type-bug']
title = 'IDLE sys.path does not contain Current Working Directory'
updated_at = <Date 2012-01-31.08:06:37.486>
user = 'https://bugs.python.org/marco'bugs.python.org fields:
activity = <Date 2012-01-31.08:06:37.486>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2012-01-31.08:06:37.487>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2011-11-30.02:51:04.496>
creator = 'marco'
dependencies = []
files = ['23816', '23846', '24377']
hgrepos = []
issue_num = 13506
keywords = ['patch']
message_count = 12.0
messages = ['148636', '148639', '148643', '148778', '148825', '148862', '149736', '152367', '152368', '152369', '152371', '152372']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'kbk', 'roger.serwy', 'python-dev', 'marco']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13506'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']
Activity
marco commentedon Nov 30, 2011
The IDLE shell sys.path does not contains any entry for the Current Working Directory ('' or '.' or '.\'); without it, when changing the CWD with os.chdir(), the shell cannot find, execute or import scripts or module in it.
I can start the standard Python interactive shell and os.chdir to any dev or test directories and import or call scripts and modules, or I can cd first to any of those directories from any shell (sh, batch, cmd.com), then start Python interactive shell and import or call scripts and modules, because there is the CWD available in the sys.path as ''.
I tried to manually add it to IDLE:
from cli by calling idle.py or idle.pyw with "-cimport sys;sys.path.insert(0,"");del sys"
or by making a IDLESTARTUP.py script
and setting the IDLESTARTUP env var pointing at it
but these work in adding the CWD to sys.path only for the first run (start) of IDLE shell; but when it get restarted (ex.: the Shell/Restart Shell toolbar option) the CWD get reset without the CWD entry, and again it has the same problem
I also tried to cd from shell (sh, bash, cmd.com) first and then start IDLE and it worked for importing and calling modules and scripts in the specific dev/test directory, but it failed to import or call other standard scripts that actually are in the sys.path (ex.: win32ui).
This is needed to use IDLE to develop and test new scripts and modules not yet ready to be included into the standard libraries and paths, so momentarily modifying the path as above that also does not survive restarts made within the application, or permanently modifying the path statically to include any dev and test dir by adding the absolute path(s ) to env var PYTHONPATH are both not correct way.
And anyway it is not consistent with the behavior of the standard Python interactive shell that includes the CWD in sys.path (as '' right at the beginning), therefore os.chdir('to any non in sys.path dirs') works correctly in contrast with IDLE shell behavior... and who knows what else it breaks.
I am not an expert on python environment, but I have 20+ years professional experience in many other high profile QA dev and testing project (just Google me), so I just thing it should be fixed, since I see this problem since 2008 and I know others that have the same issue for long time before that.
I assure you inconsistency in IDLE and Python like that separate production products from hack toys and I know for a fact are alienating both novices and veterans, because first it makes it difficult to use IDLE for basic learning and second, because it get you skeptical to trust the rest of Python will behave differently and correctly for main development, since its main IDE distributed with the language does not.
If there is anything that I don't see just let me know,
thanks.
marco commentedon Nov 30, 2011
... I think it could be fixed by adding
on the # process sys.argv and sys.path: section in PyShell.py after (~about) line 1383
(Unified diff/patch file attached)
serwy commentedon Nov 30, 2011
+1
The proposed patch works as described.
I do agree with Marco that IDLE does need some more QA.
terryjreedy commentedon Dec 2, 2011
On 3.2.2, I get >>> import sys >>> sys.path ['C:\\Programs\\Python32\\Lib\\idlelib', 'C:\\Windows\\system32\\python32.zip', 'C:\\Programs\\Python32\\DLLs', 'C:\\Programs\\Python32\\lib', 'C:\\Programs\\Python32', 'C:\\Programs\\Python32\\lib\\site-packages', 'F:\\Python']so I presume the fix should include 3.2 and 3.3.
serwy commentedon Dec 3, 2011
I have given this issue some further consideration, and realized that my "+1" recommendation was not correct.
Starting the interactive python interpreter will automatically include [''] in sys.path. A fresh restart of IDLE's shell does NOT include ''.
However, running a python script will have sys.path include the absolute path to the script. IDLE's shell does do this in conjunction with ScriptBinding's "Run Module".
The new patch preserves the existing correct behavior, but also fixes the issue.
marco commentedon Dec 5, 2011
At first I did no see the difference on preserving the existing correct behavior and fixing the issue between the two patches... and I thought less is more, so mine was better...
But, I checked again and by:
"... running a python script will have sys.path include the absolute path to the script..." (Roger meant) instead of CWD ("").
I can see the reasoning for it and since that IS the standard Python behavior it should be also transposed to IDLE.
So yes, Roger (serwy) patch presents a more accurate correction.
+1
marco commentedon Dec 18, 2011
I test Roger patch on 2.7.2.1 and 3.2.1.1 fresh installs on Win7 Pro 32bit and I confirm it is fixing the issue and it also keeps the existing correct behavior.
-----
Note:
-----
I had to apply the patch with GNU patch: Tortoise is not patching my local files (PyShell.py 2.7 and 3.2), probably because my files were not the same then the patch was diff against; both of mine are release install not SVN. Tortoise AFAIK will accept only exact patch where GNU try to use same "smart" matching.
In fact GNU patch succeeded with messages:
patching file PyShell.py
Hunk #1 succeeded at 372 with fuzz 2 (offset 7 lines).
Hunk #2 succeeded at 410 (offset 7 lines).
Hunk #3 succeeded at 438 (offset 7 lines).
Hunk #4 succeeded at 487 with fuzz 2 (offset 3 lines).
Hunk #5 succeeded at 837 (offset 2 lines).
Hunk #6 succeeded at 987 (offset 2 lines).
Hunk #7 succeeded at 1190 (offset 2 lines).
terryjreedy commentedon Jan 31, 2012
I tested on 3.2, Win 7. sys.path starts with '' on startup, after restart, and after ^c interrupt of 'while True: pass'. It start with absolute path when running a file from the editor. I believe this is as should be.
I simplified the patch a bit. Its basic idea is to add a parameter with_cwd to PyShell.ModifiedInterpreter.transfer_path. That method is called by methods start_subprocess and restart_subprocess. start_process is only called by PyShell.PyShell.begin. So I think there is no need to propagate the new parameter back past the call in start_subprocess.
.restart_subprocess is called in .poll_subprocess, .runcode, PyShell.cancel_process, and PyShell.restart_shell. (I am a little puzzled why ModifiedInterpreter.runcode calls self.interp.restart_subprocess, as in the PyShell methods, instead of self.restart_subprocess, as in poll_subprocess, but moving on...)
restart_process needs the new parameter to pass to transfer_path because it should be True for F6 Restart Shell and not for F5 Run Module. Roger's patch also left the default False for the other calls. I do not know what they do so I cannot judge them. Roger, do you know, and did you consider each? (Leaving the current behavior for those will at worst not fix something that could be, so it is safe.)
restart_shell is *bound* to F6 Restart Shell (which passes the ignored even), so the default with_cwd must be True for that binding. It is *called* by a function ScriptBinding.py that gets bound, so the call can be altered to pass the correct False. I prefer that to making two functions. That call could be replaces by a call directly to .interp.restart_subprocess(False) (and restart_shell slightly simplified).
I retested after these changes to the patch. It is from TortoiseHG, so should apply (after I modified the file paths). Since hacking on idlelib is new for me, I would like other eyes and tests before I apply.
terryjreedy commentedon Jan 31, 2012
Whoops, hit submit too soon, before saving tab to space change.
serwy commentedon Jan 31, 2012
I tested your patch and it works. For the sake of completeness, here's what I did:
Test 1: Start IDLE in shell mode and run "import sys; print(sys.path)". The first entry should be ''
This is consistent with the behavior of the regular python shell.
Test 2: Restart the shell with Ctrl+F6 (and View->Restart Shell) and repeat Test 1. This should give the same result.
Test 3: Create a script in /tmp/a.py with "import sys; print(sys.path)" as its contents. Running this script from the editor gives '/tmp' as the first entry.
This is consistent with the behavior of running "python3 /tmp/a.py".
I'd make one minor change to the patch for sake of being explicit in ScriptBinding.py:
shell.restart_shell(event, with_cwd=False)
My rationale was to modify only PyShell.py to get the correct behavior. Clearly, making a simple modification to ScriptBinding.py gives a much shorter solution. I also like your simplification for "start_subprocess".
I did consider each case of default arguments. I developed my patch iteratively starting by having "with_cwd=False" for all function calls that called "transfer_path" so that the existing (incorrect) behavior remained. I then modified how these functions were called, setting "with_cwd=True" in the cases where '' needed to be transferred, namely on the shell start and restart from within the PyShell window. That is why I introduced "restart_shell_interactive". I do agree with your reasoning that one function call is better than two.
python-dev commentedon Jan 31, 2012
New changeset 1b5abba0c808 by Terry Jan Reedy in branch '2.7':
bpo-13506 Add '' to path for interactive interpreter by adding with_cwd parameter
http://hg.python.org/cpython/rev/1b5abba0c808
New changeset 1993aa091d89 by Terry Jan Reedy in branch '3.2':
bpo-13506 Add '' to path for interactive interpreter by adding with_cwd parameter
http://hg.python.org/cpython/rev/1993aa091d89
New changeset acedd92086c5 by Terry Jan Reedy in branch 'default':
Merge 3.2
http://hg.python.org/cpython/rev/acedd92086c5
terryjreedy commentedon Jan 31, 2012
I noticed that the script in ScriptBinding.py already dereferences interp and calls 3 interp functions directly, so I decided that calling a 4th on interp was fine. That lets restart_shell be specialized to the one purpose of being bound to Restart Shell on the menu. I added a docstring to that effect so no will will try to generalize it again to use when running a module.