Skip to content

Commit d451113

Browse files
Added fix for silent failure when output file is in nonexistent dir.
Previously if you say 'gnuplot.pngfigure("nonexistent_dir/foo.png")' this would silently fail - an error message is generated in gnuplot but we cannot read anything back via the readonly PipeFile. We now treat this as an error if trying to write to a nonexistent directory.
1 parent d356f63 commit d451113

File tree

2 files changed

+64
-0
lines changed

2 files changed

+64
-0
lines changed

gnuplot.lua

+12
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,18 @@ local function filefigure(fname,term,n)
732732
if not _gptable.hasrefresh then
733733
print('Plotting to files is disabled in gnuplot 4.2, install gnuplot 4.4')
734734
end
735+
736+
-- Check whether the output directory can be written to here - torch.PipeFile
737+
-- has no read/write option so we can't read back any error messages from
738+
-- gnuplot, and writing to 'non_existent_dir/plot.png' fails silently.
739+
local outputDir = paths.dirname(fname)
740+
if outputDir == '' then
741+
outputDir = '.'
742+
end
743+
if not paths.dirp(outputDir) then
744+
error('cannot save to ' .. fname .. ': directory does not exist')
745+
end
746+
735747
local gp = getfigure(n)
736748
gp.fname = fname
737749
gp.term = term

test.lua

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
require 'gnuplot'
2+
require 'os'
3+
require 'paths'
4+
require 'torch'
5+
6+
local tester = torch.Tester()
7+
local tests = {}
8+
9+
-- Returns a random string of lowercase digits
10+
local function randomFilenameStr()
11+
local t = {}
12+
for i = 1, 10 do
13+
table.insert(t, string.char(math.random(97, 122)))
14+
end
15+
return table.concat(t)
16+
end
17+
18+
-- Make sure we can write to a new filename, but not to a nonexistent directory.
19+
function tests.cannotWriteToNonExistentDir()
20+
-- Save locally, this should work
21+
local validFilename = randomFilenameStr() .. '.png'
22+
23+
-- If this already exists (bad luck!), don't let the test overwrite it
24+
assert(not (paths.filep(validFilename) or
25+
paths.dirp(validFilename)),
26+
'random filename aready exists (?)')
27+
28+
-- Should work fine
29+
gnuplot.pngfigure(validFilename)
30+
gnuplot.plot({'Sin Curve',torch.sin(torch.linspace(-5,5))})
31+
gnuplot.plotflush()
32+
33+
-- Clean up after ourselves
34+
os.remove(validFilename)
35+
36+
-- Now make an invalid output
37+
local nonExistentDir = randomFilenameStr()
38+
assert(not (paths.filep(nonExistentDir) or
39+
paths.dirp(nonExistentDir)),
40+
'random dir aready exists (?)')
41+
42+
-- This makes an absolute path below cwd, seems Lua has no way (?) to query
43+
-- the file separator charater by itself...
44+
local invalidFilename = paths.concat(nonExistentDir, validFilename)
45+
local function shouldCrash()
46+
gnuplot.pngfigure(invalidFilename)
47+
end
48+
tester:assertErrorPattern(shouldCrash, 'directory does not exist')
49+
end
50+
51+
tester:add(tests)
52+
return tester:run()

0 commit comments

Comments
 (0)