Repository: hoburg/gpkit
Pull request #1121 (2017-06-27 22:45:51)

small issue fixes
@whoburg, ready for review

whoburg: Will review. Weird, what's going on with `macys_Sierra,cvxopt`? It's failing on a call to mosek??
whoburg: I'll find this a little easier to review if we split the Python 3 compatibility (which touches a huge number of files) into a separate PR. I'm going to make that change now -- 

 - Will rebase this to drop the two Python 3 related commits and force push
 - will open a new PR with the two Python 3 commits
whoburg: just force pushed, update local copies of this branch
bqpd: good point/plan re: splitting this!
bqpd: test models please
bqpd: (models failure with mosek_cli macys and reynolds which I can't reproduce)
test models please
bqpd: the failures are semi-reproducible: turbofan_engine_flight_profile_fleet_py_mosek_cli returns `NEAR_DUAL_FEAS`

However, in the first trial a solve with MOSEK also failed...

Perhaps we should allow for NEAR_DUAL_FEAS etc to be treated like OPTIMAL?
whoburg: I'm against passing it through as `OPTIMAL`, but I'm ok with allowing it with a printed warning.
whoburg: argh, this somehow got the python3 fixes again, should I rebase again?
whoburg: I just force pushed -- @bqpd please delete your local copy of this branch and re-fetch it from origin, so we can avoid those python3 commits creeping back on
bqpd: Yup, my bad. You kept the new small fixes I'd added?

On Jul 2, 2017 7:02 PM, "Warren Hoburg" <notifications@github.com> wrote:

> I just force pushed -- @bqpd <https://github.com/bqpd> please delete your
> local copy of this branch and re-fetch it from origin, so we can avoid
> those python3 commits creeping back on
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/hoburg/gpkit/pull/1121#issuecomment-312533250>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ABagGKe0BJuqbiDH-KHARQAOI49Tyeqzks5sKEungaJpZM4OHSQz>
> .
>

whoburg: yep, I assume you mean 6bcd34b7904c78184b45861f874d6fd018445f86, it's in the commit hx
bqpd: test models please

Patch:
diff --git a/docs/source/advancedcommands.rst b/docs/source/advancedcommands.rst
index f6ad664..a18f013 100644
--- a/docs/source/advancedcommands.rst
+++ b/docs/source/advancedcommands.rst
@@ -178,10 +178,19 @@ You can also substitute in sweep variables (see Sweeps_), strings, and monomials
 
 Note that units are preserved, and that the value can be either a string (in which case it just renames the variable), a varkey (in which case it changes its description, including the name) or a Monomial (in which case it substitutes for the variable with a new monomial).
 
+Updating ConstraintSet substitutions
+------------------------------------
+ConstraintSets have a ``.substitutions`` KeyDict attribute which will be substituted before solving.
+This KeyDict accepts variable names, VarKeys, and Variable objects as keys, and can be updated (or deleted from)
+like a regular Python dictionary to change the substitutions that will be used at solve-time. If a ConstraintSet itself
+contains ConstraintSets, it and all its elements share pointers to the same substitutions dictionary object,
+so that updating any one of them will update all of them.
+
+
 Substituting with replacement
 ------------------------------
 
-Any of the substitutions above can be run with ``p.sub(*args, replace=True)`` to clobber any previously-substituted values.
+Any of the substitutions above can be run with ``p.subinplace(*args)`` to substitute directly into the object in question.
 
 Fixed Variables
 ---------------
diff --git a/gpkit/constraints/geometric_program.py b/gpkit/constraints/geometric_program.py
index 2f5ce95..456764d 100644
--- a/gpkit/constraints/geometric_program.py
+++ b/gpkit/constraints/geometric_program.py
@@ -197,9 +197,9 @@ class GeometricProgram(CostedConstraintSet, NomialData):
                 (solvername, solver_status))
 
         if solver_status.lower() == "near_dual_feas":
-            print RuntimeWarning(
-                "final status of solver '%s' was '%s', not 'optimal'.\n\n" %
-                (solvername, solver_status))
+            print(RuntimeWarning(
+                "final status of solver '%s' was '%s', not 'optimal'.\n\n"
+                % (solvername, solver_status)))
 
         self._generate_nula(solver_out)
         self.result = self._compile_result(solver_out)  # NOTE: SIDE EFFECTS
diff --git a/gpkit/constraints/set.py b/gpkit/constraints/set.py
index 136ff89..4ddb6e6 100644
--- a/gpkit/constraints/set.py
+++ b/gpkit/constraints/set.py
@@ -44,6 +44,7 @@ class ConstraintSet(list):
                         # so we can catch them (see above) in ConstraintSets
             if hasattr(self[i], "substitutions"):
                 self.substitutions.update(self[i].substitutions)
+                self[i].substitutions = self.substitutions
         self.reset_varkeys()
         if substitutions:
             self.substitutions.update(substitutions)
@@ -183,22 +184,22 @@ class ConstraintSet(list):
                     yield yielded_constraint
 
     def subinplace(self, subs):
-        "Substitutes in place."
+        """Substitutes in place, updating self.substitutions accordingly.
+
+        Keys substituted with `subinplace` are no longer present, so if such a
+        key is also in self.substitutions that substitution is now orphaned. If
+        `subs[key]` describes some key in the ConstraintSet (i.e. one key has
+        been substituted for another), then a substitution is added, mapping
+        the orphaned value to this new key; otherwise, an error is raised.
+        """
         subs = {k.key: getattr(v, "key", v) for k, v in subs.items()}
         for constraint in self:
             constraint.subinplace(subs)
-        for key in subs:
-            if key not in self.substitutions:
-                continue
-            if hasattr(subs[key], "key"):
-                self.substitutions[subs[key]] = self.substitutions[key]
+        for key, value in subs.items():
+            if key in self.substitutions:
+                valkey, _ = self.substitutions.parse_and_index(value)
+                self.substitutions[valkey] = self.substitutions[key]
                 del self.substitutions[key]
-            else:
-                raise ValueError("the substitution {%s: %s} is invalidated"
-                                 " by the subinplace {%s: %s}, because"
-                                 " %s does not have a `key` attribute"
-                                 % (key, self.substitutions[key],
-                                    key, subs[key], subs[key]))
         self.unique_varkeys = frozenset(subs[vk] if vk in subs else vk
                                         for vk in self.unique_varkeys)
         self.reset_varkeys()
@@ -214,7 +215,9 @@ class ConstraintSet(list):
     def as_posyslt1(self, substitutions=None):
         "Returns list of posynomials which must be kept <= 1"
         posylist, self.posymap = [], []
-        for constraint in self:
+        for i, constraint in enumerate(self):
+            if not hasattr(constraint, "as_posyslt1"):
+                raise_badelement(self, i, constraint)
             posys = constraint.as_posyslt1(substitutions)
             self.posymap.append(len(posys))
             posylist.extend(posys)
diff --git a/gpkit/nomials/nomial_core.py b/gpkit/nomials/nomial_core.py
index 974366d..249139e 100644
--- a/gpkit/nomials/nomial_core.py
+++ b/gpkit/nomials/nomial_core.py
@@ -2,8 +2,8 @@
 from .data import NomialData
 from ..small_classes import Numbers
 from ..small_scripts import nomial_latex_helper
-from ..small_scripts import mag, unitstr
-from ..repr_conventions import _str, _repr, _repr_latex_
+from ..small_scripts import mag
+from ..repr_conventions import _str, _repr, _repr_latex_, unitstr
 
 
 class Nomial(NomialData):
@@ -15,6 +15,7 @@ class Nomial(NomialData):
     __str__ = _str
     __repr__ = _repr
     _repr_latex_ = _repr_latex_
+    unitstr = unitstr
 
     def str_without(self, excluded=None):
         "String representation excluding fields ('units', varkey attributes)"
@@ -38,7 +39,7 @@ class Nomial(NomialData):
                 cstr = [cstr] if (cstr != "1" or not varstrs) else []
                 mstrs.append("*".join(cstr + varstrs))
         if "units" not in excluded:
-            units = unitstr(self.units, " [%s]")
+            units = self.unitstr(" [%s]")
         else:
             units = ""
         return " + ".join(sorted(mstrs)) + units
@@ -61,7 +62,7 @@ class Nomial(NomialData):
         if "units" in excluded:
             return " + ".join(sorted(mstrs))
 
-        units = unitstr(self.units, r"\mathrm{~\left[ %s \right]}", "L~")
+        units = self.unitstr(r"\mathrm{~\left[ %s \right]}", "L~")
         units_tf = units.replace("frac", "tfrac").replace(r"\cdot", r"\cdot ")
         return " + ".join(sorted(mstrs)) + units_tf
 
diff --git a/gpkit/nomials/variables.py b/gpkit/nomials/variables.py
index 41a3719..2c45ab7 100644
--- a/gpkit/nomials/variables.py
+++ b/gpkit/nomials/variables.py
@@ -72,6 +72,11 @@ class Variable(Monomial):
         "a Variable's descr is derived from its VarKey."
         return self.key.descr
 
+    def to(self, arg):
+        "Create new Signomial converted to new units"
+         # pylint: disable=no-member
+        return Monomial(self).to(arg)
+
     def sub(self, *args, **kwargs):
         """Same as nomial substitution, but also allows single-argument calls
 
diff --git a/gpkit/repr_conventions.py b/gpkit/repr_conventions.py
index c6c7ce1..918df4d 100644
--- a/gpkit/repr_conventions.py
+++ b/gpkit/repr_conventions.py
@@ -1,4 +1,5 @@
 "Repository for representation standards"
+from .small_classes import Quantity
 
 
 def _repr(self):
@@ -14,3 +15,16 @@ def _str(self):
 def _repr_latex_(self):
     "Returns default latex for automatic iPython Notebook rendering."
     return "$$"+self.latex()+"$$"
+
+
+def unitstr(units, into="%s", options="~", dimless=""):
+    "Returns the string corresponding to an object's units."
+    if hasattr(units, "units") and isinstance(units.units, Quantity):
+        units = units.units
+    if not isinstance(units, Quantity):
+        return dimless
+    rawstr = (u"{:%s}" % options).format(units.units)
+    if str(units.units) == "count":
+        rawstr = u"count"  # TODO: remove when pint issue #356 is resolved
+    units = rawstr.replace(" ", "").replace("dimensionless", dimless)
+    return into % units or dimless
diff --git a/gpkit/small_scripts.py b/gpkit/small_scripts.py
index a3bee72..1124695 100644
--- a/gpkit/small_scripts.py
+++ b/gpkit/small_scripts.py
@@ -1,6 +1,5 @@
 """Assorted helper methods"""
 from collections import Iterable
-from .small_classes import Strings, Quantity
 
 
 def try_str_without(item, excluded):
@@ -24,24 +23,6 @@ def mag(c):
     return getattr(c, "magnitude", c)
 
 
-def unitstr(units, into="%s", options="~", dimless=""):
-    "Returns the unitstr of a given object."
-    if hasattr(units, "descr") and hasattr(units.descr, "get"):
-        units = units.descr.get("units", None)
-    if hasattr(units, "units") and isinstance(units.units, Quantity):
-        units = units.units
-    if isinstance(units, Strings):
-        return into % units or dimless
-    elif isinstance(units, Quantity):
-        rawstr = (u"{:%s}" % options).format(units.units)
-        if str(units.units) == "count":
-            # TODO remove this conditional when pint issue 356 is resolved
-            rawstr = u"count"
-        units = rawstr.replace(" ", "").replace("dimensionless", dimless)
-        return into % units or dimless
-    return dimless
-
-
 def nomial_latex_helper(c, pos_vars, neg_vars):
     """Combines (varlatex, exponent) tuples,
     separated by positive vs negative exponent,
diff --git a/gpkit/solution_array.py b/gpkit/solution_array.py
index 7dba9b4..7f4fea2 100644
--- a/gpkit/solution_array.py
+++ b/gpkit/solution_array.py
@@ -3,7 +3,8 @@ from collections import Iterable
 import numpy as np
 from .nomials import NomialArray
 from .small_classes import DictOfLists
-from .small_scripts import unitstr, mag
+from .small_scripts import mag
+from .repr_conventions import unitstr
 
 
 def senss_table(data, showvars=(), title="Sensitivities", **kwargs):
@@ -302,7 +303,7 @@ def results_table(data, title, minval=0, printunits=True, fixedcols=True,
                                   model + r"}} \\"])
             oldmodel = model
         label = var.descr.get('label', '')
-        units = var.unitstr() if printunits else ""
+        units = var.unitstr(" [%s] ") if printunits else ""
         if isvector:
             vals = [vecfmt % v for v in mag(val).flatten()[:4]]
             ellipsis = " ..." if len(val) > 4 else ""
diff --git a/gpkit/varkey.py b/gpkit/varkey.py
index a163469..1b3fd5c 100644
--- a/gpkit/varkey.py
+++ b/gpkit/varkey.py
@@ -1,6 +1,7 @@
 """Defines the VarKey class"""
 from .small_classes import Strings, Quantity, Count
-from .small_scripts import unitstr, veckeyed
+from .small_scripts import veckeyed
+from .repr_conventions import unitstr
 
 
 class VarKey(object):
@@ -96,13 +97,11 @@ class VarKey(object):
     def __getattr__(self, attr):
         return self.descr.get(attr, None)
 
-    def unitstr(self, dimless=""):
-        "Returns string representation of units"
-        return unitstr(self, into=" [%s] ", dimless=dimless)
+    unitstr = unitstr
 
     def latex_unitstr(self):
         "Returns latex unitstr"
-        us = unitstr(self.units, r"~\mathrm{%s}", "L~")
+        us = self.unitstr(r"~\mathrm{%s}", "L~")
         utf = us.replace("frac", "tfrac").replace(r"\cdot", r"\cdot ")
         return utf if utf != r"~\mathrm{-}" else ""
 


Test patch:
diff --git a/gpkit/tests/t_small.py b/gpkit/tests/t_small.py
index 40befbd..93c02f9 100644
--- a/gpkit/tests/t_small.py
+++ b/gpkit/tests/t_small.py
@@ -1,7 +1,7 @@
 """Tests for small_classes.py and small_scripts.py"""
 import unittest
 from gpkit.small_classes import HashVector, CootMatrix
-from gpkit.small_scripts import unitstr
+from gpkit.repr_conventions import unitstr
 import gpkit
 
 
@@ -69,9 +69,10 @@ class TestSmallScripts(unittest.TestCase):
         x = gpkit.Variable("x", "ft")
         # pint issue 356
         footstrings = ("ft", "foot")  # backwards compatibility with pint 0.6
-        self.assertEqual(unitstr(gpkit.Variable("n", "count")), "count")
-        self.assertIn(unitstr(x), footstrings)
-        self.assertIn(unitstr(x.key), footstrings)
+        if gpkit.units:
+            self.assertEqual(unitstr(gpkit.Variable("n", "count")), "count")
+            self.assertIn(unitstr(x), footstrings)
+            self.assertIn(unitstr(x.key), footstrings)
         self.assertEqual(unitstr(gpkit.Variable("y"), dimless="---"), "---")
         self.assertEqual(unitstr(None, dimless="--"), "--")
 
diff --git a/gpkit/tests/t_sub.py b/gpkit/tests/t_sub.py
index 56919e8..c5a8671 100644
--- a/gpkit/tests/t_sub.py
+++ b/gpkit/tests/t_sub.py
@@ -135,7 +135,7 @@ class TestModelSubs(unittest.TestCase):
         self.assertAlmostEqual(m.solve(verbosity=0)["cost"], 4, 5)
         # bad
         m = Model(x, [y >= 1], {y: 2})
-        with self.assertRaises(ValueError):
+        with self.assertRaises((KeyError, ValueError)):
             m.subinplace({y: 3})
 
     def test_bad_gp_sub(self):
diff --git a/gpkit/tests/t_vars.py b/gpkit/tests/t_vars.py
index 992ae7b..5ad2eee 100644
--- a/gpkit/tests/t_vars.py
+++ b/gpkit/tests/t_vars.py
@@ -76,7 +76,6 @@ class TestVarKey(unittest.TestCase):
         for vk in (VarKey(), x, VarKey(x), VarKey(units='m')):
             self.assertTrue(hasattr(vk, 'units'))
 
-
 class TestVariable(unittest.TestCase):
     """TestCase for the Variable class"""
 
@@ -120,6 +119,11 @@ class TestVariable(unittest.TestCase):
         y = Variable("y", "(m/s)^-0.5")
         self.assertEqual(x.units, y.units)
 
+    def test_to(self):
+        if gpkit.units:
+            x = Variable("x", "ft")
+            self.assertEqual(x.to("inch").c.magnitude, 12)
+
 
 class TestVectorVariable(unittest.TestCase):
     """TestCase for the VectorVariable class.