WIP axes are compulsory, so don't accomodate missing axes. Raise error if document is missing axes.

We know what the default location is before we start reading the masters and there is no guessing.
findDefault() method: default master is at all default values of all axes, or if that fails, the master with the copyInfo flag.
Warn if a location contains an axis that is not defined in the axes element. Otherwise ignore this value, no guessing.
Remove rulesToFeature() function.

Lib/fontTools/designspaceLib/__init__.py

@@ -6,18 +6,17 @@ import logging
 import os
 import posixpath
 import plistlib
+import warnings
+
 try:
     import xml.etree.cElementTree as ET
 except ImportError:
     import xml.etree.ElementTree as ET
-# from mutatorMath.objects.location import biasFromLocations, Location
 
 """
     designSpaceDocument
 
     - read and write designspace files
-    - axes must be defined.
-    - warpmap is stored in its axis element
 """
 
 __all__ = [
@@ -726,8 +725,7 @@ class BaseDocReader(object):
         # read the axes elements, including the warp map.
         axes = []
         if len(self.root.findall(".axes/axis"))==0:
-            self.guessAxes()
+            raise DesignSpaceDocumentError("No axes defined.")
-            self._strictAxisNames = False
             return
         for axisElement in self.root.findall(".axes/axis"):
             axisObject = self.axisDescriptorClass()
@@ -750,10 +748,13 @@ class BaseDocReader(object):
                     axisObject.labelNames[lang] = labelName
             self.documentObject.axes.append(axisObject)
             self.axisDefaults[axisObject.name] = axisObject.default
+        self.documentObject.defaultLoc = self.axisDefaults
 
     def _locationFromElement(self, locationElement):
         # mostly duplicated from readLocationElement, Needs Resolve.
         loc = {}
+        # make sure all locations start with the defaults
+        loc.update(self.axisDefaults)
         for dimensionElement in locationElement.findall(".dimension"):
             dimName = dimensionElement.attrib.get("name")
             xValue = yValue = None
@@ -774,42 +775,42 @@ class BaseDocReader(object):
                 loc[dimName] = xValue
         return loc
 
-    def guessAxes(self):
+    # def guessAxes(self):
-        # Called when we have no axes element in the file.
+    #     # Called when we have no axes element in the file.
-        # Look at all locations and collect the axis names and values
+    #     # Look at all locations and collect the axis names and values
-        # assumptions:
+    #     # assumptions:
-        # look for the default value on an axis from a master location
+    #     # look for the default value on an axis from a master location
-        # Needs deprecation warning
+    #     # Needs deprecation warning
-        allLocations = []
+    #     allLocations = []
-        minima = {}
+    #     minima = {}
-        maxima = {}
+    #     maxima = {}
-        for locationElement in self.root.findall(".sources/source/location"):
+    #     for locationElement in self.root.findall(".sources/source/location"):
-            allLocations.append(self._locationFromElement(locationElement))
+    #         allLocations.append(self._locationFromElement(locationElement))
-        for locationElement in self.root.findall(".instances/instance/location"):
+    #     for locationElement in self.root.findall(".instances/instance/location"):
-            allLocations.append(self._locationFromElement(locationElement))
+    #         allLocations.append(self._locationFromElement(locationElement))
-        for loc in allLocations:
+    #     for loc in allLocations:
-            for dimName, value in loc.items():
+    #         for dimName, value in loc.items():
-                if not isinstance(value, tuple):
+    #             if not isinstance(value, tuple):
-                    value = [value]
+    #                 value = [value]
-                for v in value:
+    #             for v in value:
-                    if dimName not in minima:
+    #                 if dimName not in minima:
-                        minima[dimName] = v
+    #                     minima[dimName] = v
-                        continue
+    #                     continue
-                    if minima[dimName] > v:
+    #                 if minima[dimName] > v:
-                        minima[dimName] = v
+    #                     minima[dimName] = v
-                    if dimName not in maxima:
+    #                 if dimName not in maxima:
-                        maxima[dimName] = v
+    #                     maxima[dimName] = v
-                        continue
+    #                     continue
-                    if maxima[dimName] < v:
+    #                 if maxima[dimName] < v:
-                        maxima[dimName] = v
+    #                     maxima[dimName] = v
-        newAxes = []
+    #     newAxes = []
-        for axisName in maxima.keys():
+    #     for axisName in maxima.keys():
-            a = self.axisDescriptorClass()
+    #         a = self.axisDescriptorClass()
-            a.default = a.minimum = minima[axisName]
+    #         a.default = a.minimum = minima[axisName]
-            a.maximum = maxima[axisName]
+    #         a.maximum = maxima[axisName]
-            a.name = axisName
+    #         a.name = axisName
-            a.tag, a.labelNames = tagForAxisName(axisName)
+    #         a.tag, a.labelNames = tagForAxisName(axisName)
-            self.documentObject.axes.append(a)
+    #         self.documentObject.axes.append(a)
 
     def readSources(self):
         for sourceCount, sourceElement in enumerate(self.root.findall(".sources/source")):
@@ -874,10 +875,8 @@ class BaseDocReader(object):
         for dimensionElement in locationElement.findall(".dimension"):
             dimName = dimensionElement.attrib.get("name")
             if self._strictAxisNames and dimName not in self.axisDefaults:
-                # In case the document contains axis definitions,
+                # In case the document contains no axis definitions,
-                # then we should only read the axes we know about.
+                warnings.warn("Location with undefined axis: \"%s\"." % dimName)
-                # However, if the document does not contain axes,
-                # then we need to create them after reading.
                 continue
             xValue = yValue = None
             try:
@@ -1094,6 +1093,7 @@ class DesignSpaceDocument(object):
         self.filename = os.path.basename(path)
         reader = self.readerClass(path, self)
         reader.read()
+        self.findDefault()
 
     def write(self, path):
         self.path = path
@@ -1222,78 +1222,23 @@ class DesignSpaceDocument(object):
                 return axisDescriptor
         return None
 
-    def check(self):
+    def findDefault(self):
-        """
+        # new default finder
-            After reading we need to make sure we have a valid designspace.
+        # take the master with the location at all the defaults
-            This means making repairs if things are missing
+        self.default = None
-                - check if we have axes and deduce them from the masters if they're missing
+        for sourceDescriptor in self.sources:
-                - that can include axes referenced in masters, instances, glyphs.
+            if sourceDescriptor.location == self.defaultLoc:
-                - if no default is assigned, use mutatormath to find out.
+                # we choose you!
-                - record the default in the designspace
+                self.default = sourceDescriptor
-                - report all the changes in a log
+                return sourceDescriptor
-                - save a "repaired" version of the doc
+        # failing that, tkae the master with the copyInfo flag
-        """
-        self.checkAxes()
-        self.checkDefault()
-
-    def checkDefault(self):
-        """ Check the sources for a copyInfo flag."""
-        flaggedDefaultCandidate = None
         for sourceDescriptor in self.sources:
             names = set()
             if sourceDescriptor.copyInfo:
                 # we choose you!
-                flaggedDefaultCandidate = sourceDescriptor
+                self.default = sourceDescriptor
-        mutatorDefaultCandidate = self.getMutatorDefaultCandidate()
+                return sourceDescriptor
-        # what are we going to do?
+        # failing that, well, fail. We don't want to guess any more. 
-        if flaggedDefaultCandidate is not None:
-            if mutatorDefaultCandidate is not None:
-                if mutatorDefaultCandidate.name != flaggedDefaultCandidate.name:
-                    # warn if we have a conflict
-                    self.logger.info("Note: conflicting default masters:\n\tUsing %s as default\n\tMutator found %s" % (flaggedDefaultCandidate.name, mutatorDefaultCandidate.name))
-            self.default = flaggedDefaultCandidate
-            self.defaultLoc = self.default.location
-        else:
-            # we have no flagged default candidate
-            # let's use the one from mutator
-            if flaggedDefaultCandidate is None and mutatorDefaultCandidate is not None:
-                # we didn't have a flag, use the one selected by mutator
-                self.default = mutatorDefaultCandidate
-                self.defaultLoc = self.default.location
-        self.default.copyInfo = True
-        # now that we have a default, let's check if the axes are ok
-        for axisObj in self.axes:
-            if axisObj.name not in self.default.location:
-                # extend the location of the neutral master with missing default value for this axis
-                self.default.location[axisObj.name] = axisObj.default
-            else:
-                if axisObj.default == self.default.location.get(axisObj.name):
-                    continue
-                # proposed remedy: change default value in the axisdescriptor to the value of the neutral
-                neutralAxisValue = self.default.location.get(axisObj.name)
-                # make sure this value is between the min and max
-                if axisObj.minimum <= neutralAxisValue <= axisObj.maximum:
-                    # yes we can fix this
-                    axisObj.default = neutralAxisValue
-                    self.logger.info("Note: updating the default value of axis %s to neutral master at %3.3f" % (axisObj.name, neutralAxisValue))
-                # always fit the axis dimensions to the location of the designated neutral
-                elif neutralAxisValue < axisObj.minimum:
-                    axisObj.default = neutralAxisValue
-                    axisObj.minimum = neutralAxisValue
-                elif neutralAxisValue > axisObj.maximum:
-                    axisObj.maximum = neutralAxisValue
-                    axisObj.default = neutralAxisValue
-                else:
-                    # now we're in trouble, can't solve this, alert.
-                    self.logger.info("Warning: mismatched default value for axis %s and neutral master. Master value outside of axis bounds" % (axisObj.name))
-
-    def getMutatorDefaultCandidate(self):
-        # FIXME: original implementation using MutatorMath
-        # masterLocations = [src.location for src in self.sources]
-        # mutatorBias = biasFromLocations(masterLocations, preferOrigin=False)
-        # for src in self.sources:
-        #     if src.location == mutatorBias:
-        #         return src
         return None
 
     def _prepAxesForBender(self):
@@ -1360,7 +1305,6 @@ class DesignSpaceDocument(object):
             a.tag, a.labelNames = tagForAxisName(a.name)
             self.logger.info("CheckAxes: added a missing axis %s, %3.3f %3.3f", a.name, a.minimum, a.maximum)
 
-
     def normalizeLocation(self, location):
         # scale this location based on the axes
         # accept only values for the axes that we have definitions for
@@ -1439,20 +1383,3 @@ class DesignSpaceDocument(object):
                 newConditionSets.append(newConditions)
             rule.conditionSets = newConditionSets
 
-
-def rulesToFeature(doc, whiteSpace="\t", newLine="\n"):
-    """ Showing how rules could be expressed as FDK feature text.
-        Speculative. Experimental.
-    """
-    axisNames = {axis.name: axis.tag for axis in doc.axes}
-    axisDims = {axis.tag: (axis.minimum, axis.maximum) for axis in doc.axes}
-    text = []
-    for rule in doc.rules:
-        text.append("rule %s{" % rule.name)
-        for cd in rule.conditions:
-            axisTag = axisNames.get(cd.get('name'), "****")
-            axisMinimum = cd.get('minimum', axisDims.get(axisTag, [0,0])[0])
-            axisMaximum = cd.get('maximum', axisDims.get(axisTag, [0,0])[1])
-            text.append("%s%s %f %f;" % (whiteSpace, axisTag, axisMinimum, axisMaximum))
-        text.append("} %s;" % rule.name)
-    return newLine.join(text)

Tests/designspaceLib/designspace_test.py

@@ -31,6 +31,29 @@ def test_fill_document(tmpdir):
     instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo")
     instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo")
     doc = DesignSpaceDocument()
+
+    # write some axes
+    a1 = AxisDescriptor()
+    a1.minimum = 0
+    a1.maximum = 1000
+    a1.default = 0
+    a1.name = "weight"
+    a1.tag = "wght"
+    # note: just to test the element language, not an actual label name recommendations.
+    a1.labelNames[u'fa-IR'] = u"قطر"
+    a1.labelNames[u'en'] = u"Wéíght"
+    doc.addAxis(a1)
+    a2 = AxisDescriptor()
+    a2.minimum = 0
+    a2.maximum = 1000
+    a2.default = 20
+    a2.name = "width"
+    a2.tag = "wdth"
+    a2.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)]
+    a2.hidden = True
+    a2.labelNames[u'fr'] = u"Chasse"
+    doc.addAxis(a2)
+
     # add master 1
     s1 = SourceDescriptor()
     s1.filename = os.path.relpath(masterPath1, os.path.dirname(testDocPath))
@@ -107,45 +130,6 @@ def test_fill_document(tmpdir):
     doc.filename = "suggestedFileName.designspace"
     doc.lib['com.coolDesignspaceApp.previewSize'] = 30
 
-    # now we have sources and instances, but no axes yet.
-    doc.check()
-
-    # Here, since the axes are not defined in the document, but instead are
-    # infered from the locations of the instances, we cannot guarantee the
-    # order in which they will be created by the `check()` method.
-    assert set(doc.getAxisOrder()) == set(['spooky', 'weight', 'width'])
-    doc.axes = []   # clear the axes
-
-    # write some axes
-    a1 = AxisDescriptor()
-    a1.minimum = 0
-    a1.maximum = 1000
-    a1.default = 0
-    a1.name = "weight"
-    a1.tag = "wght"
-    # note: just to test the element language, not an actual label name recommendations.
-    a1.labelNames[u'fa-IR'] = u"قطر"
-    a1.labelNames[u'en'] = u"Wéíght"
-    doc.addAxis(a1)
-    a2 = AxisDescriptor()
-    a2.minimum = 0
-    a2.maximum = 1000
-    a2.default = 20
-    a2.name = "width"
-    a2.tag = "wdth"
-    a2.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)]
-    a2.hidden = True
-    a2.labelNames[u'fr'] = u"Chasse"
-    doc.addAxis(a2)
-    # add an axis that is not part of any location to see if that works
-    a3 = AxisDescriptor()
-    a3.minimum = 333
-    a3.maximum = 666
-    a3.default = 444
-    a3.name = "spooky"
-    a3.tag = "SPOK"
-    a3.map = [(0.0, 10.0), (401.0, 66.0), (1000.0, 990.0)]
-    #doc.addAxis(a3)    # uncomment this line to test the effects of default axes values
     # write some rules
     r1 = RuleDescriptor()
     r1.name = "named.rule.1"
@@ -163,23 +147,11 @@ def test_fill_document(tmpdir):
     new = DesignSpaceDocument()
     new.read(testDocPath)
 
-    new.check()
     assert new.default.location == {'width': 20.0, 'weight': 0.0}
     assert new.filename == 'test.designspace'
     assert new.lib == doc.lib
     assert new.instances[0].lib == doc.instances[0].lib
 
-    # >>> for a, b in zip(doc.instances, new.instances):
-    # ...     a.compare(b)
-    # >>> for a, b in zip(doc.sources, new.sources):
-    # ...     a.compare(b)
-    # >>> for a, b in zip(doc.axes, new.axes):
-    # ...     a.compare(b)
-    # >>> [n.mutedGlyphNames for n in new.sources]
-    # [['A', 'Z'], []]
-    # >>> doc.getFonts()
-    # []
-
     # test roundtrip for the axis attributes and data
     axes = {}
     for axis in doc.axes:
@@ -197,50 +169,6 @@ def test_fill_document(tmpdir):
         assert a == b
 
 
-def test_adjustAxisDefaultToNeutral(tmpdir):
-    tmpdir = str(tmpdir)
-    testDocPath = os.path.join(tmpdir, "testAdjustAxisDefaultToNeutral.designspace")
-    masterPath1 = os.path.join(tmpdir, "masters", "masterTest1.ufo")
-    masterPath2 = os.path.join(tmpdir, "masters", "masterTest2.ufo")
-    instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo")
-    instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo")
-    doc = DesignSpaceDocument()
-    # add master 1
-    s1 = SourceDescriptor()
-    s1.filename = os.path.relpath(masterPath1, os.path.dirname(testDocPath))
-    s1.name = "master.ufo1"
-    s1.copyInfo = True
-    s1.copyFeatures = True
-    s1.location = dict(weight=55, width=1000)
-    doc.addSource(s1)
-    # write some axes
-    a1 = AxisDescriptor()
-    a1.minimum = 0
-    a1.maximum = 1000
-    a1.default = 0      # the wrong value
-    a1.name = "weight"
-    a1.tag = "wght"
-    doc.addAxis(a1)
-    a2 = AxisDescriptor()
-    a2.minimum = -10
-    a2.maximum = 10
-    a2.default = 0      # the wrong value
-    a2.name = "width"
-    a2.tag = "wdth"
-    doc.addAxis(a2)
-    # write the document
-    doc.write(testDocPath)
-    assert os.path.exists(testDocPath)
-    # import it again
-    new = DesignSpaceDocument()
-    new.read(testDocPath)
-    new.check()
-    loc = new.default.location
-    for axisObj in new.axes:
-        n = axisObj.name
-        assert axisObj.default == loc.get(n)
-
-
 def test_unicodes(tmpdir):
     tmpdir = str(tmpdir)
     testDocPath = os.path.join(tmpdir, "testUnicodes.designspace")
@@ -457,7 +385,7 @@ def test_handleNoAxes(tmpdir):
     doc.addInstance(i1)
 
     doc.write(testDocPath)
-    __removeAxesFromDesignSpace(testDocPath)
+    #__removeAxesFromDesignSpace(testDocPath)
     verify = DesignSpaceDocument()
     verify.read(testDocPath)
     verify.write(testDocPath2)
@@ -476,8 +404,16 @@ def test_pathNameResolve(tmpdir):
     instancePath1 = os.path.join(tmpdir, "instances", "instanceTest1.ufo")
     instancePath2 = os.path.join(tmpdir, "instances", "instanceTest2.ufo")
 
+    a1 = AxisDescriptor()
+    a1.tag = "TAGA"
+    a1.name = "axisName_a"
+    a1.minimum = 0
+    a1.maximum = 1000
+    a1.default = 0
+
     # Case 1: filename and path are both empty. Nothing to calculate, nothing to put in the file.
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = None
     s.path = None
@@ -494,6 +430,7 @@ def test_pathNameResolve(tmpdir):
 
     # Case 2: filename is empty, path points somewhere: calculate a new filename.
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = None
     s.path = masterPath1
@@ -510,6 +447,7 @@ def test_pathNameResolve(tmpdir):
 
     # Case 3: the filename is set, the path is None.
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = "../somewhere/over/the/rainbow.ufo"
     s.path = None
@@ -528,6 +466,7 @@ def test_pathNameResolve(tmpdir):
 
     # Case 4: the filename points to one file, the path points to another. The path takes precedence.
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = "../somewhere/over/the/rainbow.ufo"
     s.path = masterPath1
@@ -543,6 +482,7 @@ def test_pathNameResolve(tmpdir):
 
     # Case 5: the filename is None, path has a value, update the filename
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = None
     s.path = masterPath1
@@ -557,6 +497,7 @@ def test_pathNameResolve(tmpdir):
 
     # Case 6: the filename has a value, path has a value, update the filenames with force
     doc = DesignSpaceDocument()
+    doc.addAxis(a1)
     s = SourceDescriptor()
     s.filename = "../somewhere/over/the/rainbow.ufo"
     s.path = masterPath1
@@ -802,6 +743,22 @@ def test_incompleteRule(tmpdir):
     tmpdir = str(tmpdir)
     testDocPath1 = os.path.join(tmpdir, "testIncompleteRule.designspace")
     doc = DesignSpaceDocument()
+    # write some axes
+    a1 = AxisDescriptor()
+    a1.tag = "TAGA"
+    a1.name = "axisName_a"
+    a1.minimum = 0
+    a1.maximum = 1000
+    a1.default = 0
+    doc.addAxis(a1)
+    a2 = AxisDescriptor()
+    a2.tag = "TAGB"
+    a2.name = "axisName_b"
+    a2.minimum = 0
+    a2.maximum = 3000
+    a2.default = 0
+    doc.addAxis(a2)
+
     r1 = RuleDescriptor()
     r1.name = "incomplete.rule.1"
     r1.conditionSets.append([