From 4a436b54709b4d6a530cb2931e6f51a17c97945b Mon Sep 17 00:00:00 2001
From: Ronny Pfannschmidt <opensource@ronnypfannschmidt.de>
Date: Tue, 2 Oct 2018 08:03:58 +0200
Subject: [PATCH] resolve in code review commments

---
 src/_pytest/pathlib.py | 21 +++++++++++++--------
 src/_pytest/tmpdir.py  | 33 +++++++++++++++++++++++++--------
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py
index cd9796973..439f4d9ba 100644
--- a/src/_pytest/pathlib.py
+++ b/src/_pytest/pathlib.py
@@ -89,16 +89,22 @@ def parse_num(maybe_num):
         return -1
 
 
-def _max(iterable, default):
-    """needed due to python2.7 lacking the default argument for max"""
-    return reduce(max, iterable, default)
+if six.PY2:
+
+    def _max(iterable, default):
+        """needed due to python2.7 lacking the default argument for max"""
+        return reduce(max, iterable, default)
+
+
+else:
+    _max = max
 
 
 def make_numbered_dir(root, prefix):
     """create a directory with a increased number as suffix for the given prefix"""
     for i in range(10):
         # try up to 10 times to create the folder
-        max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1)
+        max_existing = _max(map(parse_num, find_suffixes(root, prefix)), default=-1)
         new_number = max_existing + 1
         new_path = root.joinpath("{}{}".format(prefix, new_number))
         try:
@@ -109,9 +115,8 @@ def make_numbered_dir(root, prefix):
             return new_path
     else:
         raise EnvironmentError(
-            "could not create numbered dir with prefix {prefix} in {root})".format(
-                prefix=prefix, root=root
-            )
+            "could not create numbered dir with prefix "
+            "{prefix} in {root} after 10 tries".format(prefix=prefix, root=root)
         )
 
 
@@ -191,7 +196,7 @@ def try_cleanup(path, consider_lock_dead_if_created_before):
 
 def cleanup_candidates(root, prefix, keep):
     """lists candidates for numbered directories to be removed - follows py.path"""
-    max_existing = _max(map(parse_num, find_suffixes(root, prefix)), -1)
+    max_existing = _max(map(parse_num, find_suffixes(root, prefix)), default=-1)
     max_delete = max_existing - keep
     paths = find_prefixed(root, prefix)
     paths, paths2 = itertools.tee(paths)
diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py
index 65562db4d..422f1f7fb 100644
--- a/src/_pytest/tmpdir.py
+++ b/src/_pytest/tmpdir.py
@@ -17,7 +17,9 @@ from .pathlib import (
 
 @attr.s
 class TempPathFactory(object):
-    """docstring for ClassName"""
+    """Factory for temporary directories under the common base temp directory.
+
+    The base directory can be configured using the ``--basetemp`` option."""
 
     given_basetemp = attr.ib()
     trace = attr.ib()
@@ -25,11 +27,15 @@ class TempPathFactory(object):
 
     @classmethod
     def from_config(cls, config):
+        """
+        :param config: a pytest configuration
+        """
         return cls(
             given_basetemp=config.option.basetemp, trace=config.trace.get("tmpdir")
         )
 
     def mktemp(self, basename, numbered=True):
+        """makes a temporary directory managed by the factory"""
         if not numbered:
             p = self.getbasetemp().joinpath(basename)
             p.mkdir()
@@ -64,12 +70,12 @@ class TempPathFactory(object):
 
 @attr.s
 class TempdirFactory(object):
-    """Factory for temporary directories under the common base temp directory.
-
-    The base directory can be configured using the ``--basetemp`` option.
+    """
+    backward comptibility wrapper that implements
+    :class:``py.path.local`` for :class:``TempPathFactory``
     """
 
-    tmppath_factory = attr.ib()
+    _tmppath_factory = attr.ib()
 
     def ensuretemp(self, string, dir=1):
         """ (deprecated) return temporary directory path with
@@ -86,13 +92,13 @@ class TempdirFactory(object):
         If ``numbered``, ensure the directory is unique by adding a number
         prefix greater than any existing one.
         """
-        return py.path.local(self.tmppath_factory.mktemp(basename, numbered).resolve())
+        return py.path.local(self._tmppath_factory.mktemp(basename, numbered).resolve())
 
     def getbasetemp(self):
-        return py.path.local(self.tmppath_factory.getbasetemp().resolve())
+        return py.path.local(self._tmppath_factory.getbasetemp().resolve())
 
     def finish(self):
-        self.tmppath_factory.trace("finish")
+        self._tmppath_factory.trace("finish")
 
 
 def get_user():
@@ -150,4 +156,15 @@ def tmpdir(request, tmpdir_factory):
 
 @pytest.fixture
 def tmp_path(tmpdir):
+    """Return a temporary directory path object
+    which is unique to each test function invocation,
+    created as a sub directory of the base temporary
+    directory.  The returned object is a :class:`pathlib.Path`
+    object.
+
+    .. note::
+
+        in python < 3.6 this is a pathlib2.Path
+    """
+
     return Path(tmpdir)