Re: [Gnuplot-py-users] [Patch] Add capability to save GridData and Data to files

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)
>  
>