|
From: Michael H. <mh...@al...> - 2005-12-13 11:55:13
|
Matthew Fulmer wrote:
> Attached is a patch that adds the keyword parameter 'filename'
> to the functions GridData and Data. Passing this parameter saves
> the passed data to a permanent file instead of a temporary file
> or a fifo. Note that I also changed the name of the class
> _TempFileItem to _NewFileItem since it now handles all non-fifo
> file creation, not just temporary files.
>
> Please CC me in your reply; I was unable to subscribe to the
> list due to (I believe) the current Sourceforge flux.
Thanks for the patch! I think that this is a good idea in general, and
I'd like to incorporate it into Gnuplot.py. But there are a few things
to be fixed before I could merge your change:
1. Please use spaces instead of tabs for indenting Python code, for
consistency with the rest of the code. And please limit line lengths to
79 characters and uniformly use 4 spaces for indenting.
2. I think that code like this
> def __init__(self, content, **keyw):
> + if keyw.has_key('filename'):
> + filename = keyw['filename']
> + del keyw['filename']
> + if filename:
> + self.temp = False
> + else:
> + filename = tempfile.mktemp()
> + self.temp = True
> + else:
> + filename = tempfile.mktemp()
> + self.temp = True
can be more succinctly written like this:
> def __init__(self, content, filename=None, **keyw):
> + if filename:
> + self.temp = False
> + else:
> + filename = tempfile.mktemp()
> + self.temp = True
3. Please include a patch to the test.py script to test the new feature.
4. Please include a patch to NEWS.txt telling what's new.
Also see notes below within your patch.
> ------------------------------------------------------------------------
>
> Index: PlotItems.py
> ===================================================================
> RCS file: /cvsroot/gnuplot-py/gnuplot-py/PlotItems.py,v
> retrieving revision 2.16
> diff -u -p -r2.16 PlotItems.py
> --- PlotItems.py 7 Nov 2005 21:10:00 -0000 2.16
> +++ PlotItems.py 10 Dec 2005 08:11:21 -0000
> @@ -302,6 +302,7 @@ class _FileItem(PlotItem):
>
> self.filename = filename
>
> + # Use single-quotes so that pgnuplot can handle DOS filenames:
> apply(PlotItem.__init__, (self,), keyw)
>
> def get_base_command_string(self):
I don't understand this comment in this context.
> @@ -336,8 +337,20 @@ class _FileItem(PlotItem):
> self._options['binary'] = (0, None)
>
>
> -class _TempFileItem(_FileItem):
> +class _NewFileItem(_FileItem):
> def __init__(self, content, **keyw):
> + if keyw.has_key('filename'):
> + filename = keyw['filename']
> + del keyw['filename']
> + if filename:
> + self.temp = False
> + else:
> + filename = tempfile.mktemp()
> + self.temp = True
> + else:
> + filename = tempfile.mktemp()
> + self.temp = True
> +
> binary = keyw.get('binary', 0)
> if binary:
> mode = 'wb'
I don't see how this part of the patch can work, as filename is
overwritten a few lines later.
> @@ -359,13 +372,14 @@ class _TempFileItem(_FileItem):
>
> # If the user hasn't specified a title, set it to None so
> # that the name of the temporary file is not used:
> - if not keyw.has_key('title'):
> + if not keyw.has_key('title') and self.temp:
> keyw['title'] = None
>
> apply(_FileItem.__init__, (self, filename,), keyw)
>
> def __del__(self):
> - os.unlink(self.filename)
> + if self.temp:
> + os.unlink(self.filename)
>
>
> class _InlineFileItem(_FileItem):
> @@ -529,6 +543,8 @@ def Data(*set, **keyw):
> rather than through a temporary file. The default is the
> value of gp.GnuplotOpts.prefer_inline_data.
>
> + 'filename=<string>' -- save data to a permanent file
> +
> The keyword arguments recognized by '_FileItem' can also be used
> here.
>
> @@ -558,7 +574,17 @@ def Data(*set, **keyw):
> cols = (cols,)
> set = Numeric.take(set, cols, -1)
>
> + if keyw.has_key('filename'):
> + filename = keyw['filename']
> + if not filename:
> + del keyw['filename']
> + filename = None
> + else:
> + filename = None
> +
> if keyw.has_key('inline'):
> + if filename:
> + raise Errors.OptionError('cannot pass data both inline and via a file')
> inline = keyw['inline']
> del keyw['inline']
> else:
I think that the same OptionError test needs to be done a few lines
later, if gp.GnuplotOpts.prefer_inline_data is set. Or, better yet,
invert the test to check for filename before considering the inline option:
if filename:
if keyw.get('inline', 0):
raise ...
elif keyw.has_key('inline'):
inline = keyw['inline']
del keyw['inline']
else:
inline = gp.GnuplotOpts.prefer_inline_data
> @@ -570,10 +596,10 @@ def Data(*set, **keyw):
> content = f.getvalue()
> if inline:
> return apply(_InlineFileItem, (content,), keyw)
> - elif gp.GnuplotOpts.prefer_fifo_data:
> + elif (not filename) and gp.GnuplotOpts.prefer_fifo_data:
> return apply(_FIFOFileItem, (content,), keyw)
> else:
> - return apply(_TempFileItem, (content,), keyw)
> + return apply(_NewFileItem, (content,), keyw)
>
>
> def GridData(data, xvals=None, yvals=None, inline=_unset, **keyw):
> @@ -596,6 +622,8 @@ def GridData(data, xvals=None, yvals=Non
>
> 'inline=<bool>' -- send data to gnuplot "inline"?
>
> + 'filename=<string>' -- save data to a permanent file
> +
> Note the unusual argument order! The data are specified *before*
> the x and y values. (This inconsistency was probably a mistake;
> after all, the default xvals and yvals are not very useful.)
> @@ -658,8 +686,22 @@ def GridData(data, xvals=None, yvals=Non
> binary = keyw.get('binary', 1) and gp.GnuplotOpts.recognizes_binary_splot
> keyw['binary'] = binary
>
> + if keyw.has_key('filename'):
> + filename = keyw['filename']
> + if not filename:
> + del keyw['filename']
> + filename = None
> + else:
> + filename = None
> +
> + binary = keyw.get('binary', 1) and gp.GnuplotOpts.recognizes_binary_splot
> + keyw['binary'] = binary
> +
> if inline is _unset:
> - inline = (not binary) and gp.GnuplotOpts.prefer_inline_data
> + inline = (not binary) and (not filename) and gp.GnuplotOpts.prefer_inline_data
> + elif inline:
> + if filename:
> + raise Errors.OptionError('cannot pass data both inline and via a file')
>
> # xvals, yvals, and data are now all filled with arrays of data.
> if binary:
> @@ -686,10 +728,10 @@ def GridData(data, xvals=None, yvals=Non
> mout[1:,1:] = Numeric.transpose(data.astype(Numeric.Float32))
>
> content = mout.tostring()
> - if gp.GnuplotOpts.prefer_fifo_data:
> + if (not filename) and gp.GnuplotOpts.prefer_fifo_data:
> return apply(_FIFOFileItem, (content,), keyw)
> else:
> - return apply(_TempFileItem, (content,), keyw)
> + return apply(_NewFileItem, (content,), keyw)
> else:
> # output data to file as "x y f(x)" triplets. This
> # requires numy copies of each x value and numx copies of
> @@ -709,9 +751,9 @@ def GridData(data, xvals=None, yvals=Non
>
> if inline:
> return apply(_InlineFileItem, (content,), keyw)
> - elif gp.GnuplotOpts.prefer_fifo_data:
> + elif (not filename) and gp.GnuplotOpts.prefer_fifo_data:
> return apply(_FIFOFileItem, (content,), keyw)
> else:
> - return apply(_TempFileItem, (content,), keyw)
> + return apply(_NewFileItem, (content,), keyw)
>
>
|