Update history.py to use pathlib

Tests were returning exeption in atexit_operations
AttributeError: 'str' object has no attribute 'unlink'

This commit embraces usage of pathlib in history.py and related
tests, fixing aforementioned problem.

IPython/core/history.py

@@ -6,7 +6,7 b''
 
 import atexit
 import datetime
-import os
+from pathlib import Path
 import re
 import sqlite3
 import threading
@@ -16,8 +16,8 b' from decorator import decorator'
 from IPython.utils.decorators import undoc
 from IPython.paths import locate_profile
 from traitlets import (
-    Any, Bool, Dict, Instance, Integer, List, Unicode, TraitError,
-    default, observe,
+    Any, Bool, Dict, Instance, Integer, List, Unicode, Union, TraitError,
+    default, observe
 )
 
 #-----------------------------------------------------------------------------
@@ -73,17 +73,18 b' def catch_corrupt_db(f, self, *a, **kw):'
             if self._corrupt_db_counter > self._corrupt_db_limit:
                 self.hist_file = ':memory:'
                 self.log.error("Failed to load history too many times, history will not be saved.")
-            elif os.path.isfile(self.hist_file):
+            elif self.hist_file.is_file():
                 # move the file out of the way
-                base, ext = os.path.splitext(self.hist_file)
-                size = os.stat(self.hist_file).st_size
+                base = str(self.hist_file.parent / self.hist_file.stem)
+                ext = self.hist_file.suffix
+                size = self.hist_file.stat().st_size
                 if size >= _SAVE_DB_SIZE:
                     # if there's significant content, avoid clobbering
                     now = datetime.datetime.now().isoformat().replace(':', '.')
                     newpath = base + '-corrupt-' + now + ext
                     # don't clobber previous corrupt backups
                     for i in range(100):
-                        if not os.path.isfile(newpath):
+                        if not Path(newpath).exists():
                             break
                         else:
                             newpath = base + '-corrupt-' + now + (u'-%i' % i) + ext
@@ -91,7 +92,7 b' def catch_corrupt_db(f, self, *a, **kw):'
                     # not much content, possibly empty; don't worry about clobbering
                     # maybe we should just delete it?
                     newpath = base + '-corrupt' + ext
-                os.rename(self.hist_file, newpath)
+                self.hist_file.rename(newpath)
                 self.log.error("History file was moved to %s and a new file created.", newpath)
             self.init_db()
             return []
@@ -128,7 +129,7 b' class HistoryAccessor(HistoryAccessorBase):'
     _corrupt_db_limit = 2
 
     # String holding the path to the history file
-    hist_file = Unicode(
+    hist_file = Union([Instance(Path), Unicode()],
         help="""Path to file to use for SQLite history database.
         
         By default, IPython will put the history database in the IPython
@@ -144,7 +145,8 b' class HistoryAccessor(HistoryAccessorBase):'
         you can also use the specific value `:memory:` (including the colon
         at both end but not the back ticks), to avoid creating an history file.
         
-        """).tag(config=True)
+        """
+    ).tag(config=True)
     
     enabled = Bool(True,
         help="""enable the SQLite history
@@ -176,7 +178,7 b' class HistoryAccessor(HistoryAccessorBase):'
                     (self.__class__.__name__, new)
             raise TraitError(msg)
     
-    def __init__(self, profile='default', hist_file=u'', **traits):
+    def __init__(self, profile='default', hist_file="", **traits):
         """Create a new history accessor.
         
         Parameters
@@ -197,7 +199,9 b' class HistoryAccessor(HistoryAccessorBase):'
         if hist_file:
             self.hist_file = hist_file
         
-        if self.hist_file == u'':
+        try:
+            self.hist_file 
+        except TraitError:
             # No one has set the hist_file, yet.
             self.hist_file = self._get_hist_file_name(profile)
         
@@ -214,7 +218,7 b' class HistoryAccessor(HistoryAccessorBase):'
         profile : str
           The name of a profile which has a history file.
         """
-        return os.path.join(locate_profile(profile), 'history.sqlite')
+        return Path(locate_profile(profile)) / 'history.sqlite'
     
     @catch_corrupt_db
     def init_db(self):
@@ -461,7 +465,7 b' class HistoryManager(HistoryAccessor):'
     @default('dir_hist')
     def _dir_hist_default(self):
         try:
-            return [os.getcwd()]
+            return [Path.cwd()]
         except OSError:
             return []
 
@@ -531,7 +535,7 b' class HistoryManager(HistoryAccessor):'
         The profile parameter is ignored, but must exist for compatibility with
         the parent class."""
         profile_dir = self.shell.profile_dir.location
-        return os.path.join(profile_dir, 'history.sqlite')
+        return Path(profile_dir)/'history.sqlite'
     
     @only_when_enabled
     def new_session(self, conn=None):
@@ -564,7 +568,7 b' class HistoryManager(HistoryAccessor):'
         optionally open a new session."""
         self.output_hist.clear()
         # The directory history can't be completely empty
-        self.dir_hist[:] = [os.getcwd()]
+        self.dir_hist[:] = [Path.cwd()]
         
         if new_session:
             if self.session_number:

IPython/core/tests/test_history.py

@@ -7,7 +7,7 b''
 
 # stdlib
 import io
-import os
+from pathlib import Path
 import sys
 import tempfile
 from datetime import datetime
@@ -29,8 +29,9 b' def test_proper_default_encoding():'
 def test_history():
     ip = get_ipython()
     with TemporaryDirectory() as tmpdir:
+        tmp_path = Path(tmpdir)
         hist_manager_ori = ip.history_manager
-        hist_file = os.path.join(tmpdir, 'history.sqlite')
+        hist_file = tmp_path / 'history.sqlite'
         try:
             ip.history_manager = HistoryManager(shell=ip, hist_file=hist_file)
             hist = [u'a=1', u'def f():\n    test = 1\n    return test', u"b='€Æ¾÷ß'"]
@@ -55,10 +56,10 b' def test_history():'
             ip.magic('%hist 2-500')
 
             # Check that we can write non-ascii characters to a file
-            ip.magic("%%hist -f %s" % os.path.join(tmpdir, "test1"))
-            ip.magic("%%hist -pf %s" % os.path.join(tmpdir, "test2"))
-            ip.magic("%%hist -nf %s" % os.path.join(tmpdir, "test3"))
-            ip.magic("%%save %s 1-10" % os.path.join(tmpdir, "test4"))
+            ip.magic("%%hist -f %s" % (tmp_path / "test1"))
+            ip.magic("%%hist -pf %s" % (tmp_path / "test2"))
+            ip.magic("%%hist -nf %s" % (tmp_path / "test3"))
+            ip.magic("%%save %s 1-10" % (tmp_path / "test4"))
 
             # New session
             ip.history_manager.reset()
@@ -126,8 +127,8 b' def test_history():'
             nt.assert_equal(list(gothist), [(1,3,(hist[2],"spam"))] )
 
             # Cross testing: check that magic %save can get previous session.
-            testfilename = os.path.realpath(os.path.join(tmpdir, "test.py"))
-            ip.magic("save " + testfilename + " ~1/1-3")
+            testfilename = (tmp_path /"test.py").resolve()
+            ip.magic("save " + str(testfilename) + " ~1/1-3")
             with io.open(testfilename, encoding='utf-8') as testfile:
                 nt.assert_equal(testfile.read(),
                                         u"# coding: utf-8\n" + u"\n".join(hist)+u"\n")
@@ -176,13 +177,13 b' def test_timestamp_type():'
 def test_hist_file_config():
     cfg = Config()
     tfile = tempfile.NamedTemporaryFile(delete=False)
-    cfg.HistoryManager.hist_file = tfile.name
+    cfg.HistoryManager.hist_file = Path(tfile.name)
     try:
         hm = HistoryManager(shell=get_ipython(), config=cfg)
         nt.assert_equal(hm.hist_file, cfg.HistoryManager.hist_file)
     finally:
         try:
-            os.remove(tfile.name)
+            Path(tfile.name).unlink()
         except OSError:
             # same catch as in testing.tools.TempFileMixin
             # On Windows, even though we close the file, we still can't
@@ -197,7 +198,7 b' def test_histmanager_disabled():'
     ip = get_ipython()
     with TemporaryDirectory() as tmpdir:
         hist_manager_ori = ip.history_manager
-        hist_file = os.path.join(tmpdir, 'history.sqlite')
+        hist_file = Path(tmpdir) / 'history.sqlite'
         cfg.HistoryManager.hist_file = hist_file
         try:
             ip.history_manager = HistoryManager(shell=ip, config=cfg)
@@ -211,4 +212,4 b' def test_histmanager_disabled():'
             ip.history_manager = hist_manager_ori
 
     # hist_file should not be created
-    nt.assert_false(os.path.exists(hist_file))
+    nt.assert_false(hist_file.exists())

IPython/testing/tools.py

@@ -10,6 +10,7 b' Authors'
 # Distributed under the terms of the Modified BSD License.
 
 import os
+from pathlib import Path
 import re
 import sys
 import tempfile
@@ -142,7 +143,7 b' def default_config():'
     config.TerminalTerminalInteractiveShell.term_title = False,
     config.TerminalInteractiveShell.autocall = 0
     f = tempfile.NamedTemporaryFile(suffix=u'test_hist.sqlite', delete=False)
-    config.HistoryManager.hist_file = f.name
+    config.HistoryManager.hist_file = Path(f.name)
     f.close()
     config.HistoryManager.db_cache_size = 10000
     return config