From 48e3fd8a801eff62d1365755925f2e3b077e1ffa Mon Sep 17 00:00:00 2001
From: Hassan Kibirige <has2k1@gmail.com>
Date: Mon, 24 Dec 2018 11:48:20 +0300
Subject: [PATCH] Default to `base-2` minor breaks for log tranforms

---
 doc/changelog.rst               | 12 ++++++++++++
 mizani/tests/test_breaks.py     | 15 ++++++++++++++-
 mizani/tests/test_transforms.py |  8 +++++++-
 mizani/transforms.py            | 14 +++++++++++---
 4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/doc/changelog.rst b/doc/changelog.rst
index f42e1ac..aadf0fb 100644
--- a/doc/changelog.rst
+++ b/doc/changelog.rst
@@ -1,6 +1,18 @@
 Changelog
 =========
 
+v0.5.3
+------
+*(not-yet-released)*
+
+API Changes
+-----------
+- Log transforms now default to ``base - 2`` minor breaks.
+  So base 10 has 8 minor breaks and 9 partitions,
+  base 8 has 6 minor breaks and 7 partitions, ...,
+  base 2 has 0 minor breaks and a single partition.
+
+
 v0.5.2
 ------
 *(2018-17-10)*
diff --git a/mizani/tests/test_breaks.py b/mizani/tests/test_breaks.py
index 0828578..00fb1be 100644
--- a/mizani/tests/test_breaks.py
+++ b/mizani/tests/test_breaks.py
@@ -8,7 +8,7 @@
 from mizani.breaks import (mpl_breaks, log_breaks, minor_breaks,
                            trans_minor_breaks, date_breaks,
                            timedelta_breaks, extended_breaks)
-from mizani.transforms import trans
+from mizani.transforms import trans, log_trans
 
 
 def test_mpl_breaks():
@@ -130,6 +130,19 @@ def __init__(self):
     with pytest.raises(TypeError):
         t.minor_breaks(major)
 
+    # Test minor_breaks for log scales are 2 less than the base
+    base = 10
+    breaks = np.arange(1, 3)
+    limits = [breaks[0], breaks[-1]]
+    t = log_trans(base)
+    assert len(t.minor_breaks(breaks, limits)) == base - 2
+
+    base = 5  # Odd base
+    breaks = np.arange(1, 3)
+    limits = [breaks[0], breaks[-1]]
+    t = log_trans(base)
+    assert len(t.minor_breaks(breaks, limits)) == base - 2
+
 
 def test_date_breaks():
     # cpython
diff --git a/mizani/tests/test_transforms.py b/mizani/tests/test_transforms.py
index 56115cb..11dd080 100644
--- a/mizani/tests/test_transforms.py
+++ b/mizani/tests/test_transforms.py
@@ -63,13 +63,19 @@ def _test_trans(trans, x):
     t = gettrans(trans())
     xt = t.transform(x)
     x2 = t.inverse(xt)
+    is_log_trans = (t.__class__.__name__.startswith('log') and
+                    hasattr(t, 'base'))
     # round trip
     npt.assert_allclose(x, x2)
     major = t.breaks([min(x), max(x)])
     minor = t.minor_breaks(t.transform(major))
     # Breaks and they are finite
     assert len(major)
-    assert len(minor)
+    if is_log_trans and int(t.base) == 2:
+        # Minor breaks for base == 2
+        assert len(minor) == 0
+    else:
+        assert len(minor)
     assert all(np.isfinite(major))
     assert all(np.isfinite(minor))
     # Not breaks outside the domain
diff --git a/mizani/transforms.py b/mizani/transforms.py
index 9f536fc..fb77362 100644
--- a/mizani/transforms.py
+++ b/mizani/transforms.py
@@ -169,7 +169,7 @@ def breaks(self, limits):
 
 def trans_new(name, transform, inverse, breaks=None,
               minor_breaks=None, _format=None,
-              domain=(-np.inf, np.inf), doc=''):
+              domain=(-np.inf, np.inf), doc='', **kwargs):
     """
     Create a transformation class object
 
@@ -198,6 +198,9 @@ def trans_new(name, transform, inverse, breaks=None,
         It should be of length 2.
     doc : str
         Docstring for the class.
+    **kwargs : dict
+        Attributes of the transform, e.g if base is passed
+        in kwargs, then `t.base` would be a valied attribute.
 
     Returns
     -------
@@ -215,7 +218,8 @@ def _get(func):
     d = {'transform': _get(transform),
          'inverse': _get(inverse),
          'domain': domain,
-         '__doc__': doc}
+         '__doc__': doc,
+         **kwargs}
 
     if breaks:
         d['breaks_'] = _get(breaks)
@@ -278,12 +282,14 @@ def inverse(x):
     if 'breaks' not in kwargs:
         kwargs['breaks'] = log_breaks(base=base)
 
+    kwargs['base'] = base
     kwargs['_format'] = log_format(base)
 
     _trans = trans_new(name, transform, inverse, **kwargs)
 
     if 'minor_breaks' not in kwargs:
-        _trans.minor_breaks = trans_minor_breaks(_trans, 4)
+        n = int(base) - 2
+        _trans.minor_breaks = trans_minor_breaks(_trans, n=n)
 
     return _trans
 
@@ -327,6 +333,7 @@ def transform(x):
     def inverse(x):
         return np.log(x)/np.log(base)
 
+    kwargs['base'] = base
     return trans_new(name, transform, inverse, **kwargs)
 
 
@@ -406,6 +413,7 @@ def transform(x):
     def inverse(x):
         return (np.abs(x) * p + np.sign(x)) ** (1 / p)
 
+    kwargs['p'] = p
     kwargs['name'] = kwargs.get('name', 'pow_{}'.format(p))
     kwargs['transform'] = transform
     kwargs['inverse'] = inverse