search

Factual / drake

Data workflow tool, like a "Make for data"
Other
1.48k stars 110 forks source link

Quick Fix for #153 #154

Open andrew-christianson opened 9 years ago

andrew-christianson commented 9 years ago

In the spirit of fixing, rather than whinging, this is a quick fix for my issue #153. Splitting paths on : in drake.fs$split-path returns ('C', '\\data\\test.csv') for properly formed windows paths. This causes drake.fs$get-fs to return [fs, 'C', '\\data\\test.csv]. So, a basic function e.g. (fs di/data-in? "C:\\data\\test.csv") goes to (di/data-in? fs '\\data\\test.csv') which clearly breaks for windows.

Splitting on "://" should still get the correct prefixes for e.g. s3://bucket.blah or hdfs://... without mangling the windows paths.

Note: It seems like there may be a different (and possibly cleaner?) solution to this using make-path more liberally, but it wasn't clear to me exactly where to use that, and this worked. Hence, the quick-fix appellation.

amalloy commented 9 years ago

I don't know about this...URIs don't have to have :// necessarily: consider mailto:obama@whitehouse.gov, for example. This might be okay if all URI schemes that drake actually supports do always use ://, but I don't know enough about that to accept this pull request, which would risk breaking other stuff I don't understand.

andrew-christianson commented 9 years ago

Fair enough, and thanks for the prompt response. If I get the time, I'll look for alternative solutions.