pyfilesystem2

pyfilesystem2 copied to clipboard

I think os.expandvars needs to be called before abspath in the opener.

Example:

from fs import open_fs
>>> open_fs("~")
OSFS('/Users/tf')

>>> open_fs("$HOME")
...[snip]...
fs.errors.CreateFailed: root path '/Users/tf/Desktop/Users/tf' does not exist

I found the same unintuitive behavior on windows (open_fs("%USERPROFILE%")). The user profile path is appended to the current working dir.

You could argue that calling os.expandvars is the job of user-code, rather than pyfilesystem-code? :shrug: Because it's entirely possible to have a directory named $HOME:

$ cd /tmp
$ mkdir \$HOME
$ cd \$HOME
$ pwd
/tmp/$HOME

I see, but that's what the expand_vars option is for, isn't it? Wouldn't it be nicer if OSFS("$HOME") and open_fs("osfs://$HOME") had the same result?

Wouldn't it be nicer if OSFS("$HOME") and open_fs("osfs://$HOME") had the same result?

Oh! I hadn't realised that expand_vars defaults to True https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/osfs.py#L97

If https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/osfs.py#L125 does both os.path.expandvars and os.path.expanduser, then perhaps it makes sense for https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/opener/osfs.py#L38 to also use both, rather than only doing os.path.expanduser ? OTOH, given that there's (currently) no option to control the expand_vars behaviour for osfs:// URLs, perhaps this would be a backwards-incompatible change? :shrug:

(On a side-note, I wonder if it even makes sense for https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/opener/osfs.py#L38 to be calling os.path.expanduser, given that the OSFS init-method always calls it too?)

Side-note 2: I've not played with FS-URLs and the fs-opener stuff, but why does https://github.com/PyFilesystem/pyfilesystem2/blob/master/fs/opener/osfs.py#L38 unconditionally do join(cwd, ... ? Shouldn't it only try to prepend the cwd to the FS-URL path, if the FS-URL path is relative rather than absolute? :shrug:

Yes I wondered about that, too. Maybe expand_vars could be a parameter of fs.open_fs? That way one could pass an FTP password via an env variable.

Yes, we could probably provide expand_vars as an fs.open_fs parameter, which would however default to False for backward compatibility!

@althonos Instead of making expand_vars a parameter of fs.open_fs which is not specific to OSFS, wdyt of making the expand_vars a URL param that gets passed to OSFS in https://github.com/PyFilesystem/pyfilesystem2/blob/59f6e4d51a1983ca28d0b667bd1d6b3284ab5c56/fs/opener/osfs.py#L40

in a similar manner to FTPFS https://github.com/PyFilesystem/pyfilesystem2/blob/59f6e4d51a1983ca28d0b667bd1d6b3284ab5c56/fs/opener/ftpfs.py#L43-L51

So, something like

+          expand_vars = parse_result.params.get("expand_vars", "true").lower() == "true"
-          osfs = OSFS(path, create=create)
+          osfs = OSFS(path, create=create, expand_vars=expand_vars)