biocore / pyqi

Tools for developing and testing command line interfaces in Python.
Other
9 stars 13 forks source link

qcli: new_filepath doesn't create folders if they don't exist #267

Open ElDeveloper opened 9 years ago

ElDeveloper commented 9 years ago

If an argument is declared as type='new_filepath', and a directory that doesn't exist is passed, say 'foo/bar/baz/new-file.txt', the directories foo/bar/baz are not created. This causes some trouble with file pointers that try to write and create a new file in a non-existant path.

Originally brought up in https://github.com/biocore/qiime/issues/1994

gregcaporaso commented 9 years ago

Thanks @ElDeveloper. I think it'd make the most sense for this to just raise an informative error message, rather than do recursive directory creation. What do you think?

wasade commented 9 years ago

Recursive would match "mkdir -p" tho On Apr 29, 2015 9:03 PM, "Greg Caporaso" notifications@github.com wrote:

Thanks @ElDeveloper https://github.com/ElDeveloper. I think it'd make the most sense for this to just raise an informative error message, rather than do recursive directory creation. What do you think?

— Reply to this email directly or view it on GitHub https://github.com/biocore/pyqi/issues/267#issuecomment-97645252.

gregcaporaso commented 9 years ago

right

ElDeveloper commented 9 years ago

An informative error message before any of the processing begins is ideal! Though creating the directories matches what I would expect in a parameter that is labeled as "new_filepath", but I honestly don't have strong feelings.

On (Apr-29-15|20:12), Greg Caporaso wrote:

right


Reply to this email directly or view it on GitHub: https://github.com/biocore/pyqi/issues/267#issuecomment-97647596

gregcaporaso commented 9 years ago

I don't really have strong feelings about this either - you guys choose.

wasade commented 9 years ago

preference to mimic mkdir -p, though that has consequences. Notably that it will not err if the directories already exist:

$ mkdir test
$ mkdir -p test/1/2/3/4
$ mkdir -p test/1/2/3/4
$ mkdir -p test/1/2/3/4
$

...but, given that we're shifting from pyqi to click, I think we should minimize effort, therefore just use makedirs. The difference being that it will error if the path already exists (which I think matches expectations of new_filepath anyway):

08:12:19 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
> import os

08:12:49 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
> os.makedirs('test2/1/2/3/4')
---------------------------------------------------------------------------
OSError                                   Traceback (most recent call last)
<ipython-input-4-a7543925550e> in <module>()
----> 1 os.makedirs('test2/1/2/3/4')

/Users/mcdonadt/.virtualenvs/python27/lib/python2.7/os.pyc in makedirs(name, mode)
    155         if tail == curdir:           # xxx/newdir/. exists if xxx/newdir exists
    156             return
--> 157     mkdir(name, mode)
    158
    159 def removedirs(name):

OSError: [Errno 17] File exists: 'test2/1/2/3/4'

08:12:50 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
>
ElDeveloper commented 9 years ago

I think that makes sense. As long as when it errors, we print out a nice and informative error, I'm ok. I initially brought this up because I was trying to run group_significance.py on a table and it error with the traceback on the QIIME issue ... which I hadn't seen before and it took me a while to understand what was the problem.

On (Apr-30-15| 7:14), Daniel McDonald wrote:

preference to mimic mkdir -p, though that has consequences. Notably that it will not err if the directories already exist:

$ mkdir test
$ mkdir -p test/1/2/3/4
$ mkdir -p test/1/2/3/4
$ mkdir -p test/1/2/3/4
$

...but, given that we're shifting from pyqi to click, I think we should minimize effort, therefore just use makedirs. The difference being that it will error if the path already exists (which I think matches expectations of new_filepath anyway):

08:12:19 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
> import os

08:12:49 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
> os.makedirs('test2/1/2/3/4')
---------------------------------------------------------------------------
OSError                                   Traceback (most recent call last)
<ipython-input-4-a7543925550e> in <module>()
----> 1 os.makedirs('test2/1/2/3/4')

/Users/mcdonadt/.virtualenvs/python27/lib/python2.7/os.pyc in makedirs(name, mode)
    155         if tail == curdir:           # xxx/newdir/. exists if xxx/newdir exists
    156             return
--> 157     mkdir(name, mode)
    158
    159 def removedirs(name):

OSError: [Errno 17] File exists: 'test2/1/2/3/4'

08:12:50 (mcdonadt@Daniels-MacBook-Pro-3):/Users/mcdonadt/not_backed_up
>

Reply to this email directly or view it on GitHub: https://github.com/biocore/pyqi/issues/267#issuecomment-97811889