TST: Use tempfiles in all tests.
State: closed

If you really need to, possible to opt-out with make_tempfile=False.
Something weird with HDF where, even though it's _supposed_ to be
removing the file every time, it causes issues to use a real temp
file... Not sure why (maybe I missed something there).


Labels:

Comments:
why r u changing the hdf stuff? this is specifically designed to guarantee the file removal

Problem is that it should be creating the files in the temp directory rather than the current directory (like it is doing on master) - tried to add that in to hdf tests but somehow it wasn't working.

isn't that what make temp file is supposed to do?

Yes, but if you passed an actual filename, it didn't create a temp file.

On Sat, Nov 2, 2013 at 9:13 PM, jreback notifications@github.com wrote:

> isn't that what make temp file is supposed to do?
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5419#issuecomment-27636563
> .

maybe I messed something up with hdf.

On Sat, Nov 2, 2013 at 9:13 PM, Jeffrey Tratner
jeffrey.tratner@gmail.comwrote:

> Yes, but if you passed an actual filename, it didn't create a temp file.
> 
> On Sat, Nov 2, 2013 at 9:13 PM, jreback notifications@github.com wrote:
> 
> > isn't that what make temp file is supposed to do?
> > 
> > —
> > Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5419#issuecomment-27636563
> > .

ok let me have a look tom

I think you just need to check in ensure_clean if the path is a filename, if so then put it in a temp dir, but use that filename (otherwise generate both)

@jreback that's what I did (just by passing in as `suffix`). I'll check again - maybe there's a part that uses ensure_clean twice or something:

``` python
import tempfile
In [4]: a = tempfile.mkstemp()[1]

In [5]: b = tempfile.mkstemp(suffix='apples!')[1]

In [6]: a
Out[6]: '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/tmp7oEhoT'

In [7]: b
Out[7]: '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/tmphkXUEKapples!'

In [8]: b = tempfile.mkstemp(suffix='apples!', prefix='red')[1]

In [9]: b
Out[9]: '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/redqbTnB_apples!'

In [10]:
```

seems odd that u have to pass the make_tempfile=False in test_pytables just check if the passed filename is a a full pathname 

@jreback the point isn't to maintain the file path, it's that all the files we create for testing need to go into the temp directory. I added that make_tempfile=False part to work around pytables tests (otherwise they get littered everywhere).  For example, I get this error when I try to use it with tempfile:

``` python
ERROR: pandas.io.tests.test_pytables:TestHDFStore.test_mode
  /usr/local/bin/vim +412 pandas/io/tests/test_pytables.py  # test_mode
    check('r')
  /usr/local/bin/vim +377 pandas/io/tests/test_pytables.py  # check
    self.assertRaises(IOError, HDFStore, path, mode=mode)
  /usr/local/bin/vim +475 ../python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/case.py  # assertRaises
    callableObj(*args, **kwargs)
  /usr/local/bin/vim +384 pandas/io/pytables.py  # __init__
    self.open(mode=mode)
  /usr/local/bin/vim +504 pandas/io/pytables.py  # open
    self._handle = h5_open(self._path, self._mode)
  /usr/local/bin/vim +230 pandas/io/pytables.py  # h5_open
    return tables.openFile(path, mode)
  /usr/local/bin/vim +35  ../python2.7/site-packages/tables/_past.py  # oldfunc
    return obj(*args, **kwargs)
  /usr/local/bin/vim +251 ../python2.7/site-packages/tables/file.py  # open_file
    return File(filename, mode, title, root_uep, filters, **kwargs)
  /usr/local/bin/vim +527 ../python2.7/site-packages/tables/file.py  # __init__
    self._g_new(filename, mode, **params)
  /usr/local/bin/vim +463 hdf5extension.pyx  # tables.hdf5extension.File._g_new (tables/hdf5extension.c:4284)
    None
HDF5ExtError: HDF5 error back trace

  File "H5F.c", line 1582, in H5Fopen
    unable to open file
  File "H5F.c", line 1373, in H5F_open
    unable to read superblock
  File "H5Fsuper.c", line 334, in H5F_super_read
    unable to find file signature
  File "H5Fsuper.c", line 155, in H5F_locate_signature
    unable to find a valid file signature

End of HDF5 error back trace

Unable to open/create file '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/tmpcoEZWF__cJHxrUtdtb__.h5'
```

this particular test IS supposed to raise (as it's testing using read mode on a non-existent file)

why don't u take the hdf stuff out of this PR and I'll fix up - 

yep - if I revert the last PR that's where we end up. My goal was to make it so that new people automatically opt in to using temp files (instead of allowing a filename to determine it) + allow you to specify the file extension you want (i.e. `ensure_clean('.xls') as path` makes path end with '.xls'

okay, I pushed back to the HDF-opt-out. if you use `make_tempfile=False` you get the original behavior of ensure_clean.

@jtratner ok, see #5422, fixes some actual testing bugs....however I think ensure_clean is prob in conflict for our PR's.  don't need the `make_tempfile` kw any longer, so go ahead and merge yours w/o the hdf stuff and I will rebase 

How about I cherry-pick #5422 on to this and then put everything together? Might be simpler...

yep...go for it....makes more sense

@jreback right now 'r+' is causing a file not existing error - https://travis-ci.org/jtratner/pandas/jobs/13440462

why did you take all of the changes out from utils/testing.py???? you don't need this make_tempfile bizness

just revert the last commit on THIS pr (the hdf stuff you changed), add in my stuff and it will work

Ah okay...I just tried to cherry-pick the HEAD of your branch, that's why
it messed up.

On Sun, Nov 3, 2013 at 2:11 PM, jreback notifications@github.com wrote:

> just revert the last commit on THIS pr (the hdf stuff you changed), add in
> my stuff and it will work
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5419#issuecomment-27651492
> .

Btw - the way you set up the filename thing, if you pass a path like
"/a/b/c.txt", it will just completely ignore it:

``` python

In [1]: import pandas.util.testing as tm

In [2]: s = tm.ensure_clean('/a/b/c.txt')

In [4]: s.__enter__()
Out[4]: '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/tmpQL_8ra'

In [7]: s = tm.ensure_clean('a.txt')

In [8]: s.__enter__()
Out[8]: '/var/folders/rz/5czmf6hj3kdbhbqwxz4klf8h0000gn/T/tmpkucAwIa.txt'
```

@jtratner that was on purpose, in theory if you wanted to pass a fully qualified name, that means you actually want to use it that way (which is silly but, but not meant to be bullet proof)

you still have `make_tempfile=False` all over the place which defeats the purpose of using the `ensure_clean`, take that out

This is now at the same point as your PR, except that it raises a ValueError. It looks like it's failing with the same set of errors as the hdf_temp PR. Do you want to pick this up and iron out the problems? (and I can close this)

sure

closing in favor of #5425

unbelievable...the whole problem was that needed to use `mktemp` and not `mkstemp`; the later opens the file and even you actually close it, hdf bombs (on py3 only).....now good to go 

No no can't use mktemp - not safe and deprecated
http://docs.python.org/2/library/tempfile.html

I don't understand why it opens the file though, that's totally silly,
given that you could just use the temporary file class for that. We could
just hack together a temp directory that's a global initialized in each
file or something...