Add home directory expansion to IPYTHON_DIR environment variables.

[wilsaj]

If you set IPYTHON_DIR to '/something/' then run ipython, a '/something/' directory is created in whatever path you've called ipython from, which is probably not the expected behavior for most people.

[takluyver]

I don't think the startswith check is necessary: we should be able to just call os.path.expanduser on it.

[wilsaj]

Good point. I was thinking an unnecessary expanduser call might be worth avoiding, but it's negligible. I'll update.

[takluyver]

In fact, looking at the code, the first thing expanduser does is a startswith("~") check. But yes, in any case, it's not an expensive function, and we only call this function at startup.

Also, can you add a test for this case? Have a look at the tests already in IPython/utils/tests/test_path.py: it should be easy to copy and tweak one.

[wilsaj]

Ok, that should do it.

I cleaned things up a bit and found a bug in test_path.py. It didn't seem like those were big enough to file a separate pull request, but let me know if I went too far.

[takluyver]

That's fine - thanks for finding it. I might have done the cleanup in a separate pull request, because it makes it hard to see what code is actually changed, but it's there now, so we'll go with it.

A couple of minor things:

• You're using .rstrip('/') to take off a redundant directory separator at the end, but that's platform dependent. Will os.path.normpath do what you're after?
• For your new test function, the docstring should describe it as ..._7, not ..._2.

[wilsaj]

You're right, os.path.normpath is what I was going for.

[takluyver]

OK, then that all looks good to me. I'll test it later, unless someone else beats me to it.

[minrk]

@wilsaj thanks for the fix, but in the future please don't ever include 'cleanup' of whitespace or enforcing pep8's ludicrous line-length limit as part of a pull request that actually does something.

[wilsaj]

Sorry about that. I didn't mean to offend.

[minrk]

@wilsaj - no offense taken, I just get unnecessarily peeved with pep8 sometimes. The only real reason is that the diff gets harder to read when it's full of changes that have no effect, and whitespace fixes reduce the effectiveness of tools like git blame.

[wilsaj]

For me, pep8 makes it slightly easier to visually parse code that I'm not familiar with. So I thought there was some value in nudging things in that direction while I had these files open. I didn't realize that it would trash the history in some ways. Does a git revert fix those problems? If you merge --squash the branch, the whitespace/pep8 changes go away and git blame is clean again. But maybe you don't want to do that. I can make another branch/pull request if you'd prefer.

[minrk]

No, it's not a big enough deal to make any changes here, just for future reference. If this were really interesting, delicate code I might ask for rolling back the commit, but that's not the case with the path files. I think this is fine for merge, if @takluver says it works.

[takluyver]

Test passes, code looks sensible. Go for it.

[wilsaj]

Just a heads up: I already committed a revert of the whitespace and pep8 stuff, so if you pull and merge each individual change (without rebase or squash), you get the trashed git blame and none of the cleanup.

[minrk]

Rebased, squashed, and merged.

Thanks again.