Python 2 NamedTemporaryFile

[jayvdb]

Firstly, in tempfile.py there is

    def _infer_return_type(*args):
        _types = set()
        for arg in args:
            if isinstance(type(arg), six.string_types):
                _types.add(str)
            elif isinstance(type(arg), bytes):
                _types.add(bytes)
            elif arg:
                _types.add(type(arg))

Worth noting that the final elif arg means any use of u'' or b'' is ignored. This is different from stdlib, but probably not a bad thing. There are also other differences with this implementations vs the stdlib one, resulting in lots of errors when testing this implementation of _infer_return_type with the stdlib test class TestLowLevelInternals.

But the important part to note is that unicode input results in unicode as the inferred type. isinstance(type(u'foo'), six.string_types) is False.

os.fsencode doesnt exist on Python 2, so any use of unicode as args to the local NamedTemporaryFile will result in an error.

This also applies to create_tracked_tempfile, which passes all args through to NamedTemporaryFile.

Copying the example from the doctest in the README.rst and docs/quickstart.rst, and making them unicode_literals...

>>> temp_file = vistir.path.create_tracked_tempfile(prefix=u"requirements", suffix=u"txt")

tests/test_path.py:90:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
src/vistir/path.py:306: in create_tracked_tempfile
    return _NamedTemporaryFile(*args, **kwargs)
src/vistir/backports/tempfile.py:192: in NamedTemporaryFile
    prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

prefix = 'requirements', suffix = 'txt', dir = None

    def _sanitize_params(prefix, suffix, dir):
        """Common parameter processing for most APIs in this module."""
        output_type = _infer_return_type(prefix, suffix, dir)
        if suffix is None:
            suffix = output_type()
        if prefix is None:
            if output_type is str:
                prefix = "tmp"
            else:
                prefix = os.fsencode("tmp")
        if dir is None:
            if output_type is str:
                dir = gettempdir()
            else:
>               dir = os.fsencode(gettempdir())
E               AttributeError: 'module' object has no attribute 'fsencode'

src/vistir/backports/tempfile.py:51: AttributeError

atomic_open_for_write(u'foo/bar', ...) doesnt hit this, as it supplies a dir and a prefix.

You can partially remove this problem by substituting os.fsencode("tmp") for b'tmp' , as that is a no-brainer you dont need os.fsencode for.

Removing os.fsencode(gettempdir()) will be more difficult/intrusive, depending on the implementation goals. The main source of problems is gettempdir uses _candidate_tempdir_list() which first tries envvars TMPDIR, TEMP, and TMP, so anything is possible. It could be re-implemented to only use those envvars if they are paths which resolve correctly without using os.fsencode, perhaps falling back to the OS default paths which are saner.

Or, use backports.os.fsencode , but be aware of PiDelport/backports.os#13 , and I've also encountered other problems with it while working on PiDelport/backports.tempfile#7 , but I havent identified that problem as clearly atm, eluding to it only a code comment.

[techalchemy]

I have a reimplementation of fsencode but most of this code is just a workaround for new failures in python 3.7.2 that only occur in CI

[techalchemy]

ok so to give a better response here -- the code taken here is taken from the stdlib test utils which polls for the existence of a file while trying to remove it specifically to handle race conditions during testing. I'm not totally sure it's necessary anymore to be quite honest, but we were encountering race conditions in pipenv related to cleanup starting in 3.7.2... I'll see about removing this and whether it has upstream impact

[jayvdb]

If you do find you need a real tempfile.NamedTemporaryFile with fsencoding , we should work together with others on it in the backports packages, but if you can avoiding needing that, you safe yourself a lot of unnecessary headache ;-)