riga / law

Build large-scale task workflows: luigi + job submission + remote targets + environment sandboxing using Docker/Singularity
http://law.readthedocs.io
BSD 3-Clause "New" or "Revised" License
96 stars 39 forks source link

local target makedirs doesn't use absolute path #178

Closed cverstege closed 1 month ago

cverstege commented 2 months ago

When using a local filesystem for a local file target, makedirs() doesn't use the full path, but only the relative path from the current working directory. See:

>>> import law
>>> fs_plots = law.LocalFileSystem(base="/web/cverstege/public_html/plots/")
>>> out = law.LocalFileTarget("my/subfolder/plot.pdf", fs=fs_plots)                                                                                                                                                
>>> out.makedirs()
>>> import os
>>> os.path.exists("/web/cverstege/public_html/plots/my/subfolder/")                                                                                                                                               
False
>>> os.path.exists("my/subfolder/")
True

I would expect the following behavior:

>>> import law
>>> fs_plots = law.LocalFileSystem(base="/web/cverstege/public_html/plots/")
>>> out = law.LocalFileTarget("my/subfolder/plot.pdf", fs=fs_plots)                                                                                                                                                
>>> out.makedirs()
>>> import os
>>> os.path.exists("/web/cverstege/public_html/plots/my/subfolder/")                                                                                                                                               
True
>>> os.path.exists("my/subfolder/")
False
riga commented 2 months ago

Thanks for reporting @cverstege!

I'll have a look.

riga commented 1 month ago

Indeed. The targets in law did not overload the makedirs method coming directly from luigi's target class, but that should be fixed now in the master. (I for my part usually use target.parent.touch(), but good that you caught that :+1:)

Feel free to re-open in case you still see issues.

cverstege commented 1 month ago

Thank you for the quick fix :+1: