Repository: sympy/sympy

Issue:
empty Piecewise fails to pretty print
Consider:
```
>>> f = Piecewise()
>>> print(f)
Piecewise()
```
Ok, but:
```
>>> pprint(f)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-dd6b50590f87> in <module>()
----> 1 pprint(f)

/home/cbm/src/sympy.git/sympy/printing/pretty/pretty.pyc in pretty_print(expr, wrap_line, num_columns, use_unicode, full_prec, order, use_unicode_sqrt_char)
   2379     print(pretty(expr, wrap_line=wrap_line, num_columns=num_columns,
   2380                  use_unicode=use_unicode, full_prec=full_prec, order=order,
-> 2381                  use_unicode_sqrt_char=use_unicode_sqrt_char))
   2382 
   2383 pprint = pretty_print

/home/cbm/src/sympy.git/sympy/printing/pretty/pretty.pyc in pretty(expr, **settings)
   2339 
   2340     try:
-> 2341         return pp.doprint(expr)
   2342     finally:
   2343         pretty_use_unicode(uflag)

/home/cbm/src/sympy.git/sympy/printing/pretty/pretty.pyc in doprint(self, expr)
     60 
     61     def doprint(self, expr):
---> 62         return self._print(expr).render(**self._settings)
     63 
     64     # empty op so _print(stringPict) returns the same

/home/cbm/src/sympy.git/sympy/printing/printer.pyc in _print(self, expr, **kwargs)
    285                 printmethod = '_print_' + cls.__name__
    286                 if hasattr(self, printmethod):
--> 287                     return getattr(self, printmethod)(expr, **kwargs)
    288             # Unknown object, fall back to the emptyPrinter.
    289             return self.emptyPrinter(expr)

/home/cbm/src/sympy.git/sympy/printing/pretty/pretty.pyc in _print_Piecewise(self, pexpr)
   1038         # max widths
   1039         maxw = [max([P[i, j].width() for i in range(len_args)])
-> 1040                 for j in range(2)]
   1041 
   1042         # FIXME: Refactor this code and matrix into some tabular environment.

ValueError: max() arg is an empty sequence

>>> latex(f)

<similar error>
```

Now maybe you think no one should make empty Piecewise but they can occur naturally:
```
>>> f = Piecewise((42, x > 0)))
>>> pprint(f)
{42  for x > 0
```
`f` is undefined for non-positive numbers:
```
>>> n = Symbol('n', negative=True)
>>> print(f.subs(z, n))
Piecewise()
>>> print(f.subs(z, n))

<error as above>
```

(Another possible resolution is that empty Piecewise raise an exception.)

More background, just FYI: `octave_code(Piecewise())` and the other code generators deal with this in a graceful way by raising exception for all Piecewise that do not have a default block.

Discussion:

Patch:
diff --git a/sympy/functions/elementary/piecewise.py b/sympy/functions/elementary/piecewise.py
index 6c9230c6d5..d46ab050da 100644
--- a/sympy/functions/elementary/piecewise.py
+++ b/sympy/functions/elementary/piecewise.py
@@ -71,7 +71,7 @@ class Piecewise(Function):
           If any of the evaluated conds are not determined explicitly False,
           e.g. x < 1, the function is returned in symbolic form.
         - If the function is evaluated at a place where all conditions are False,
-          a ValueError exception will be raised.
+          nan will be returned.
         - Pairs where the cond is explicitly False, will be removed.
 
     Examples
@@ -120,6 +120,8 @@ class Piecewise(Function):
     is_Piecewise = True
 
     def __new__(cls, *args, **options):
+        if len(args) == 0:
+            raise TypeError("At least one (expr, cond) pair expected.")
         # (Try to) sympify args first
         newargs = []
         for ec in args:
@@ -153,13 +155,13 @@ def eval(cls, *_args):
         3) any repeat of a previous condition is ignored
         3) any args past one with a true condition are dropped
 
-        If there are no args left, an empty Piecewise will be returned.
+        If there are no args left, nan will be returned.
         If there is a single arg with a True condition, its
         corresponding expression will be returned.
         """
 
         if not _args:
-            return
+            return Undefined
 
         if len(_args) == 1 and _args[0][-1] == True:
             return _args[0][0]
@@ -797,13 +799,17 @@ def _eval_subs(self, old, new):
         # and avoiding invalid conditions that appear after a
         # True condition
         args = list(self.args)
+        args_exist = False
         for i, (e, c) in enumerate(args):
             c = c._subs(old, new)
             if c != False:
+                args_exist = True
                 e = e._subs(old, new)
             args[i] = (e, c)
             if c == True:
                 break
+        if not args_exist:
+            args = ((Undefined, True),)
         return self.func(*args)
 
     def _eval_transpose(self):
diff --git a/sympy/physics/units/dimensions.py b/sympy/physics/units/dimensions.py
index f4d5449122..2aabd59238 100644
--- a/sympy/physics/units/dimensions.py
+++ b/sympy/physics/units/dimensions.py
@@ -18,9 +18,7 @@
 
 from sympy import Integer, Matrix, S, Symbol, sympify, Basic, Tuple, Dict, default_sort_key
 from sympy.core.compatibility import reduce, string_types
-from sympy.core.basic import Basic
 from sympy.core.expr import Expr
-from sympy.core.power import Pow
 from sympy.utilities.exceptions import SymPyDeprecationWarning
 
 
@@ -152,25 +150,22 @@ def _register_as_base_dim(self):
         dimsys_default._args = (dimsys_default.args[:2] + (Dict(d),))
 
     def __add__(self, other):
-        from sympy.physics.units.quantities import Quantity
-        other = sympify(other)
-        if isinstance(other, Basic):
-            if other.has(Quantity):
-                other = Dimension(Quantity.get_dimensional_expr(other))
-            if isinstance(other, Dimension) and self == other:
-                return self
-            return super(Dimension, self).__add__(other)
-        return self
+        """Define the addition for Dimension.
 
-    def __radd__(self, other):
-        return self + other
+        Addition of dimension has a sense only if the second object is
+        the same dimension (we don't add length to time).
+        """
 
-    def __sub__(self, other):
-        # there is no notion of ordering (or magnitude) among dimension,
-        # subtraction is equivalent to addition when the operation is legal
-        return self + other
+        if not isinstance(other, Dimension):
+            raise TypeError("Only dimension can be added; '%s' is not valid"
+                            % type(other))
+        elif isinstance(other, Dimension) and self != other:
+            raise ValueError("Only dimension which are equal can be added; "
+                             "'%s' and '%s' are different" % (self, other))
 
-    def __rsub__(self, other):
+        return self
+
+    def __sub__(self, other):
         # there is no notion of ordering (or magnitude) among dimension,
         # subtraction is equivalent to addition when the operation is legal
         return self + other
@@ -183,22 +178,16 @@ def _eval_power(self, other):
         return Dimension(self.name**other)
 
     def __mul__(self, other):
-        from sympy.physics.units.quantities import Quantity
-        if isinstance(other, Basic):
-            if other.has(Quantity):
-                other = Dimension(Quantity.get_dimensional_expr(other))
-            if isinstance(other, Dimension):
-                return Dimension(self.name*other.name)
-            if not other.free_symbols:
-                return self
-            return super(Dimension, self).__mul__(other)
-        return self
+        if not isinstance(other, Dimension):
+            return self
 
-    def __rmul__(self, other):
-        return self*other
+        return Dimension(self.name*other.name)
 
     def __div__(self, other):
-        return self*Pow(other, -1)
+        if not isinstance(other, Dimension):
+            return self
+
+        return Dimension(self.name/other.name)
 
     def __rdiv__(self, other):
         return other * pow(self, -1)
@@ -418,31 +407,28 @@ def _get_dimensional_dependencies_for_name(self, name):
         if name.is_Number:
             return {}
 
-        get_for_name = dimsys_default._get_dimensional_dependencies_for_name
-
         if name.is_Mul:
             ret = collections.defaultdict(int)
-            dicts = [get_for_name(i) for i in name.args]
+            dicts = [dimsys_default._get_dimensional_dependencies_for_name(i) for i in name.args]
             for d in dicts:
                 for k, v in d.items():
                     ret[k] += v
             return {k: v for (k, v) in ret.items() if v != 0}
 
         if name.is_Pow:
-            dim = get_for_name(name.base)
+            dim = dimsys_default._get_dimensional_dependencies_for_name(name.base)
             return {k: v*name.exp for (k, v) in dim.items()}
 
         if name.is_Function:
             args = (Dimension._from_dimensional_dependencies(
-                get_for_name(arg)) for arg in name.args)
+                dimsys_default._get_dimensional_dependencies_for_name(arg)
+            ) for arg in name.args)
             result = name.func(*args)
 
             if isinstance(result, Dimension):
                 return dimsys_default.get_dimensional_dependencies(result)
-            elif result.func == name.func:
-                return {}
-            else:
-                return get_for_name(result)
+            # TODO shall we consider a result that is not a dimension?
+            # return Dimension._get_dimensional_dependencies_for_name(result)
 
     def get_dimensional_dependencies(self, name, mark_dimensionless=False):
         if isinstance(name, Dimension):
diff --git a/sympy/physics/units/quantities.py b/sympy/physics/units/quantities.py
index 8677f7ce57..7c6bb7c92f 100644
--- a/sympy/physics/units/quantities.py
+++ b/sympy/physics/units/quantities.py
@@ -247,3 +247,28 @@ def convert_to(self, other):
     def free_symbols(self):
         """Return free symbols from quantity."""
         return self.scale_factor.free_symbols
+
+
+def _Quantity_constructor_postprocessor_Add(expr):
+    # Construction postprocessor for the addition,
+    # checks for dimension mismatches of the addends, thus preventing
+    # expressions like `meter + second` to be created.
+
+    deset = {
+        tuple(sorted(dimsys_default.get_dimensional_dependencies(
+            Dimension(Quantity.get_dimensional_expr(i) if not i.is_number else 1
+        )).items()))
+        for i in expr.args
+        if i.free_symbols == set()  # do not raise if there are symbols
+                    # (free symbols could contain the units corrections)
+    }
+    # If `deset` has more than one element, then some dimensions do not
+    # match in the sum:
+    if len(deset) > 1:
+        raise ValueError("summation of quantities of incompatible dimensions")
+    return expr
+
+
+Basic._constructor_postprocessor_mapping[Quantity] = {
+    "Add" : [_Quantity_constructor_postprocessor_Add],
+}
diff --git a/sympy/physics/units/util.py b/sympy/physics/units/util.py
index 9175c4158b..9b520c34ee 100644
--- a/sympy/physics/units/util.py
+++ b/sympy/physics/units/util.py
@@ -132,7 +132,7 @@ def get_total_scale_factor(expr):
 
 def quantity_simplify(expr):
     """Return an equivalent expression in which prefixes are replaced
-    with numerical values and all units of a given dimension are the
+    with numerical values and products of related quantities are
     unified in a canonical manner.
 
     Examples
@@ -143,70 +143,34 @@ def quantity_simplify(expr):
     >>> from sympy.physics.units import foot, inch
     >>> quantity_simplify(kilo*foot*inch)
     250*foot**2/3
-    >>> quantity_simplify(foot - 6*inch)
-    foot/2
     """
 
-    if expr.is_Atom or not expr.has(Prefix, Quantity):
+    if expr.is_Atom:
         return expr
 
-    # replace all prefixes with numerical values
-    p = expr.atoms(Prefix)
-    expr = expr.xreplace({p: p.scale_factor for p in p})
+    if isinstance(expr, Prefix):
+        return expr.scale_factor
 
-    # replace all quantities of given dimension with a canonical
-    # quantity, chosen from those in the expression
-    d = sift(expr.atoms(Quantity), lambda i: i.dimension)
-    for k in d:
-        if len(d[k]) == 1:
-            continue
-        v = list(ordered(d[k]))
-        ref = v[0]/v[0].scale_factor
-        expr = expr.xreplace({vi: ref*vi.scale_factor for vi in v[1:]})
-
-    return expr
+    expr = expr.func(*map(quantity_simplify, expr.args))
 
+    if not expr.is_Mul or not expr.has(Quantity):
+        return expr
 
-def check_dimensions(expr):
-    """Return expr if there are not unitless values added to
-    dimensional quantities, else raise a ValueError."""
-    from sympy.solvers.solveset import _term_factors
-    # the case of adding a number to a dimensional quantity
-    # is ignored for the sake of SymPy core routines, so this
-    # function will raise an error now if such an addend is
-    # found.
-    # Also, when doing substitutions, multiplicative constants
-    # might be introduced, so remove those now
-    adds = expr.atoms(Add)
-    DIM_OF = dimsys_default.get_dimensional_dependencies
-    for a in adds:
-        deset = set()
-        for ai in a.args:
-            if ai.is_number:
-                deset.add(())
-                continue
-            dims = []
-            skip = False
-            for i in Mul.make_args(ai):
-                if i.has(Quantity):
-                    i = Dimension(Quantity.get_dimensional_expr(i))
-                if i.has(Dimension):
-                    dims.extend(DIM_OF(i).items())
-                elif i.free_symbols:
-                    skip = True
-                    break
-            if not skip:
-                deset.add(tuple(sorted(dims)))
-                if len(deset) > 1:
-                    raise ValueError(
-                        "addends have incompatible dimensions")
-
-    # clear multiplicative constants on Dimensions which may be
-    # left after substitution
-    reps = {}
-    for m in expr.atoms(Mul):
-        if any(isinstance(i, Dimension) for i in m.args):
-            reps[m] = m.func(*[
-                i for i in m.args if not i.is_number])
-
-    return expr.xreplace(reps)
+    args_pow = [arg.as_base_exp() for arg in expr.args]
+    quantity_pow, other_pow = sift(
+        args_pow, lambda x: isinstance(x[0], Quantity), binary=True)
+    coeff = Mul.fromiter([
+        Pow(b, e, evaluate=False) for b, e in other_pow])
+    quantity_pow_by_dim = sift(quantity_pow, lambda x: x[0].dimension)
+    new_quantities = []
+    for _, bp in quantity_pow_by_dim.items():
+        if len(bp) == 1:
+            new_quantities.append(bp[0][0]**bp[0][1])
+        else:
+            # just let reference quantity be the first quantity,
+            # picked from an ordered list
+            bp = list(ordered(bp))
+            ref = bp[0][0]/bp[0][0].scale_factor
+            new_quantities.append(Mul.fromiter((
+                (ref*b.scale_factor)**p for b, p in bp)))
+    return coeff*Mul.fromiter(new_quantities)


Test patch:
diff --git a/sympy/functions/elementary/tests/test_piecewise.py b/sympy/functions/elementary/tests/test_piecewise.py
index 07f2bdf659..c49adace2e 100644
--- a/sympy/functions/elementary/tests/test_piecewise.py
+++ b/sympy/functions/elementary/tests/test_piecewise.py
@@ -30,9 +30,13 @@ def test_piecewise():
         Piecewise((x, Or(x < 1, x < 2)), (0, True))
     assert Piecewise((x, x < 1), (x, x < 2), (x, True)) == x
     assert Piecewise((x, True)) == x
+    # Explicitly constructed empty Piecewise not accepted
+    raises(TypeError, lambda: Piecewise())
     # False condition is never retained
-    assert Piecewise((x, False)) == Piecewise(
-        (x, False), evaluate=False) == Piecewise()
+    assert Piecewise((2*x, x < 0), (x, False)) == \
+        Piecewise((2*x, x < 0), (x, False), evaluate=False) == \
+        Piecewise((2*x, x < 0))
+    assert Piecewise((x, False)) == Undefined
     raises(TypeError, lambda: Piecewise(x))
     assert Piecewise((x, 1)) == x  # 1 and 0 are accepted as True/False
     raises(TypeError, lambda: Piecewise((x, 2)))
@@ -84,11 +88,16 @@ def test_piecewise():
         ).subs(x, 1) == Piecewise((-1, y < 1), (2, True))
     assert Piecewise((1, Eq(x**2, -1)), (2, x < 0)).subs(x, I) == 1
 
+    p6 = Piecewise((x, x > 0))
+    n = symbols('n', negative=True)
+    assert p6.subs(x, n) == Undefined
+
     # Test evalf
     assert p.evalf() == p
     assert p.evalf(subs={x: -2}) == -1
     assert p.evalf(subs={x: -1}) == 1
     assert p.evalf(subs={x: 1}) == log(1)
+    assert p6.evalf(subs={x: -5}) == Undefined
 
     # Test doit
     f_int = Piecewise((Integral(x, (x, 0, 1)), x < 1))
diff --git a/sympy/physics/units/tests/test_dimensions.py b/sympy/physics/units/tests/test_dimensions.py
index e3998dfeec..986ff26dcd 100644
--- a/sympy/physics/units/tests/test_dimensions.py
+++ b/sympy/physics/units/tests/test_dimensions.py
@@ -5,7 +5,6 @@
 
 from sympy import S, Symbol, sqrt
 from sympy.physics.units.dimensions import Dimension, length, time, dimsys_default
-from sympy.physics.units import foot
 from sympy.utilities.pytest import raises
 
 
@@ -70,30 +69,13 @@ def test_Dimension_add_sub():
     assert length - length == length
     assert -length == length
 
-    assert length + foot == foot + length == length
-    assert length - foot == foot - length == length
-    assert length + time == length - time != length
-
-    # issue 14547 - only raise error for dimensional args; allow
-    # others to pass
-    x = Symbol('x')
-    e = length + x
-    assert e == x + length and e.is_Add and set(e.args) == {length, x}
-    e = length + 1
-    assert e == 1 + length == 1 - length and e.is_Add and set(e.args) == {length, 1}
+    raises(TypeError, lambda: length + 1)
+    raises(TypeError, lambda: length - 1)
+    raises(ValueError, lambda: length + time)
+    raises(ValueError, lambda: length - time)
 
 
 def test_Dimension_mul_div_exp():
-    assert 2*length == length*2 == length/2 == length
-    assert 2/length == 1/length
-    x = Symbol('x')
-    m = x*length
-    assert m == length*x and m.is_Mul and set(m.args) == {x, length}
-    d = x/length
-    assert d == x*length**-1 and d.is_Mul and set(d.args) == {x, 1/length}
-    d = length/x
-    assert d == length*x**-1 and d.is_Mul and set(d.args) == {1/x, length}
-
     velo = length / time
 
     assert (length * length) == length ** 2
diff --git a/sympy/physics/units/tests/test_quantities.py b/sympy/physics/units/tests/test_quantities.py
index 4b94674f7e..8649301a1a 100644
--- a/sympy/physics/units/tests/test_quantities.py
+++ b/sympy/physics/units/tests/test_quantities.py
@@ -283,7 +283,8 @@ def test_issue_quart():
 
 
 def test_issue_5565():
-    assert (m < s).is_Relational
+    raises(ValueError, lambda: m < s)
+    assert (m < km).is_Relational
 
 
 def test_find_unit():
@@ -316,6 +317,20 @@ def test_Quantity_derivative():
     assert diff(meter**2, meter) == 2*meter
 
 
+def test_sum_of_incompatible_quantities():
+    raises(ValueError, lambda: meter + 1)
+    raises(ValueError, lambda: meter + second)
+    raises(ValueError, lambda: 2 * meter + second)
+    raises(ValueError, lambda: 2 * meter + 3 * second)
+    raises(ValueError, lambda: 1 / second + 1 / meter)
+    raises(ValueError, lambda: 2 * meter*(mile + centimeter) + km)
+
+    expr = 2 * (mile + centimeter)/second + km/hour
+    assert expr in Basic._constructor_postprocessor_mapping
+    for i in expr.args:
+        assert i in Basic._constructor_postprocessor_mapping
+
+
 def test_quantity_postprocessing():
     q1 = Quantity('q1')
     q2 = Quantity('q2')
@@ -444,32 +459,3 @@ def test_eval_subs():
     expr2 = force/mass
     units = {force:gravitational_constant*kilogram**2/meter**2, mass:kilogram}
     assert expr2.subs(units) == gravitational_constant*kilogram/meter**2
-
-
-def test_issue_14932():
-    assert (log(inch) - log(2)).simplify() == log(inch/2)
-    assert (log(inch) - log(foot)).simplify() == -log(12)
-    p = symbols('p', positive=True)
-    assert (log(inch) - log(p)).simplify() == log(inch/p)
-
-
-def test_issue_14547():
-    # the root issue is that an argument with dimensions should
-    # not raise an error when the the `arg - 1` calculation is
-    # performed in the assumptions system
-    from sympy.physics.units import foot, inch
-    from sympy import Eq
-    assert log(foot).is_zero is None
-    assert log(foot).is_positive is None
-    assert log(foot).is_nonnegative is None
-    assert log(foot).is_negative is None
-    assert log(foot).is_algebraic is None
-    assert log(foot).is_rational is None
-    # doesn't raise error
-    assert Eq(log(foot), log(inch)) is not None  # might be False or unevaluated
-
-    x = Symbol('x')
-    e = foot + x
-    assert e.is_Add and set(e.args) == {foot, x}
-    e = foot + 1
-    assert e.is_Add and set(e.args) == {foot, 1}
diff --git a/sympy/physics/units/tests/test_util.py b/sympy/physics/units/tests/test_util.py
index 079404aed2..3d961f1b0f 100644
--- a/sympy/physics/units/tests/test_util.py
+++ b/sympy/physics/units/tests/test_util.py
@@ -6,17 +6,14 @@
 
 from sympy.utilities.exceptions import SymPyDeprecationWarning
 
-from sympy import (Add, Mul, Pow, Tuple, pi, sin, sqrt, sstr, sympify,
-    symbols)
+from sympy import Add, Mul, Pow, Tuple, pi, sin, sqrt, sstr, sympify
 from sympy.physics.units import (
     G, centimeter, coulomb, day, degree, gram, hbar, hour, inch, joule, kelvin,
     kilogram, kilometer, length, meter, mile, minute, newton, planck,
     planck_length, planck_mass, planck_temperature, planck_time, radians,
-    second, speed_of_light, steradian, time, km)
+    second, speed_of_light, steradian, time)
 from sympy.physics.units.dimensions import dimsys_default
-from sympy.physics.units.util import convert_to, dim_simplify, check_dimensions
-from sympy.utilities.pytest import raises
-
+from sympy.physics.units.util import convert_to, dim_simplify
 
 
 def NS(e, n=15, **options):
@@ -145,23 +142,7 @@ def test_quantity_simplify():
     x, y = symbols('x y')
 
     assert quantity_simplify(x*(8*kilo*newton*meter + y)) == x*(8000*meter*newton + y)
-    assert quantity_simplify(foot*inch*(foot + inch)) == foot**2*(foot + foot/12)/12
-    assert quantity_simplify(foot*inch*(foot*foot + inch*(foot + inch))) == foot**2*(foot**2 + foot/12*(foot + foot/12))/12
-    assert quantity_simplify(2**(foot/inch*kilo/1000)*inch) == 4096*foot/12
+    assert quantity_simplify(foot*inch*(foot + inch)) == foot**2*(foot + inch)/12
+    assert quantity_simplify(foot*inch*(foot*foot + inch*(foot + inch))) == foot**2*(foot**2 + inch*(foot + inch))/12
+    assert quantity_simplify(2**(foot/inch*kilo/1000)*inch) == 4096*inch
     assert quantity_simplify(foot**2*inch + inch**2*foot) == 13*foot**3/144
-
-
-def test_check_dimensions():
-    x = symbols('x')
-    assert check_dimensions(inch + x) == inch + x
-    assert check_dimensions(length + x) == length + x
-    # after subs we get 2*length; check will clear the constant
-    assert check_dimensions((length + x).subs(x, length)) == length
-    raises(ValueError, lambda: check_dimensions(inch + 1))
-    raises(ValueError, lambda: check_dimensions(length + 1))
-    raises(ValueError, lambda: check_dimensions(length + time))
-    raises(ValueError, lambda: check_dimensions(meter + second))
-    raises(ValueError, lambda: check_dimensions(2 * meter + second))
-    raises(ValueError, lambda: check_dimensions(2 * meter + 3 * second))
-    raises(ValueError, lambda: check_dimensions(1 / second + 1 / meter))
-    raises(ValueError, lambda: check_dimensions(2 * meter*(mile + centimeter) + km))