Merge pull request #1267 from fonttools/designspaceLib-checkDefault

designspaceLib remove checkDefault, checkAxes

Doc/source/designspaceLib/readme.rst

@@ -573,6 +573,7 @@ There are two meanings for the ``lib`` element:
     - Child element of ``designspace`` and ``instance``
     - Contains arbitrary data about the whole document or about a specific
       instance.
+    - Items in the dict need to use **reverse domain name notation** <https://en.wikipedia.org/wiki/Reverse_domain_name_notation>__
 
 .. 32-info-element:
 

Doc/source/designspaceLib/scripting.rst

@@ -59,6 +59,8 @@ Make a descriptor object and add it to the document.
 -  The ``tag`` attribute is the one of the registered `OpenType
    Variation Axis
    Tags <https://www.microsoft.com/typography/otspec/fvar.htm#VAT>`__
+-  The default master is expected at the intersection of all
+   default values of all axes. 
 
 Option: add label names
 -----------------------
@@ -123,6 +125,7 @@ So go ahead and add another master:
     s1.location = dict(weight=1000)
     doc.addSource(s1)
     
+
 Option: exclude glyphs
 ----------------------
 

Lib/fontTools/designspaceLib/__init__.py

@@ -6,18 +6,16 @@ import logging
 import os
 import posixpath
 import plistlib
+
 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__ = [
@@ -179,9 +177,7 @@ class RuleDescriptor(SimpleDescriptor):
 
 
 def evaluateRule(rule, location):
-    """ Return True if any of the rule's conditionsets matches the
+    """ Return True if any of the rule's conditionsets matches the given location."""
-    given location.
-    """
     return any(evaluateConditions(c, location) for c in rule.conditionSets)
 
 
@@ -364,14 +360,8 @@ class BaseDocWriter(object):
         self.documentVersion = "4.0"
         self.root = ET.Element("designspace")
         self.root.attrib['format'] = self.documentVersion
-        self.axes = []
+        self._axes = []     # for use by the writer only
-        self.rules = []
+        self._rules = []    # for use by the writer only
-
-    def newDefaultLocation(self):
-        loc = collections.OrderedDict()
-        for axisDescriptor in self.axes:
-            loc[axisDescriptor.name] = axisDescriptor.default
-        return loc
 
     def write(self, pretty=True):
         if self.documentObject.axes:
@@ -407,13 +397,11 @@ class BaseDocWriter(object):
         locElement = ET.Element("location")
         if name is not None:
             locElement.attrib['name'] = name
-        defaultLoc = self.newDefaultLocation()
+        validatedLocation = self.documentObject.newDefaultLocation()
-        # Without OrderedDict, output XML would be non-deterministic.
+        for axisName, axisValue in locationObject.items():
-        # https://github.com/LettError/designSpaceDocument/issues/10
+            if axisName in validatedLocation:
-        validatedLocation = collections.OrderedDict()
+                # only accept values we know
-        for axisName, axisValue in defaultLoc.items():
+                validatedLocation[axisName] = axisValue
-            # update the location dict with missing default axis values
-            validatedLocation[axisName] = locationObject.get(axisName, axisValue)
         for dimensionName, dimensionValue in validatedLocation.items():
             dimElement = ET.Element('dimension')
             dimElement.attrib['name'] = dimensionName
@@ -432,7 +420,7 @@ class BaseDocWriter(object):
 
     def _addRule(self, ruleObject):
         # if none of the conditions have minimum or maximum values, do not add the rule.
-        self.rules.append(ruleObject)
+        self._rules.append(ruleObject)
         ruleElement  = ET.Element('rule')
         if ruleObject.name is not None:
             ruleElement.attrib['name'] = ruleObject.name
@@ -451,9 +439,6 @@ class BaseDocWriter(object):
                 conditionsetElement.append(conditionElement)
             if len(conditionsetElement):
                 ruleElement.append(conditionsetElement)
-        # XXX shouldn't we require at least one sub element?
-        # if not ruleObject.subs:
-        #     raise DesignSpaceDocument('Invalid empty rule with no "sub" elements')
         for sub in ruleObject.subs:
             subElement = ET.Element('sub')
             subElement.attrib['name'] = sub[0]
@@ -463,7 +448,7 @@ class BaseDocWriter(object):
             self.root.findall('.rules')[0].append(ruleElement)
 
     def _addAxis(self, axisObject):
-        self.axes.append(axisObject)
+        self._axes.append(axisObject)
         axisElement = ET.Element('axis')
         axisElement.attrib['tag'] = axisObject.tag
         axisElement.attrib['name'] = axisObject.name
@@ -657,12 +642,13 @@ class BaseDocReader(object):
         tree = ET.parse(self.path)
         self.root = tree.getroot()
         self.documentObject.formatVersion = self.root.attrib.get("format", "3.0")
-        self.axes = []
+        self._axes = []
         self.rules = []
         self.sources = []
         self.instances = []
         self.axisDefaults = {}
         self._strictAxisNames = True
+        self.logger = logging.getLogger("DesignSpaceLog")
 
     def read(self):
         self.readAxes()
@@ -677,21 +663,32 @@ class BaseDocReader(object):
             paths.append(self.documentObject.sources[name][0].path)
         return paths
 
-    def newDefaultLocation(self):
-        loc = {}
-        for axisDescriptor in self.axes:
-            loc[axisDescriptor.name] = axisDescriptor.default
-        return loc
-
     def readRules(self):
-        # read the rules
+        # we also need to read any conditions that are outside of a condition set.
         rules = []
         for ruleElement in self.root.findall(".rules/rule"):
             ruleObject = self.ruleDescriptorClass()
             ruleObject.name = ruleElement.attrib.get("name")
+            # read any stray conditions outside a condition set
+            externalConditions = self._readConditionElements(ruleElement)
+            if externalConditions:
+                ruleObject.conditionSets.append(externalConditions)
+                self.logger.info('Found stray rule conditions outside a conditionset. Wrapped them in a new conditionset.')
+            # read the conditionsets
             for conditionSetElement in ruleElement.findall('.conditionset'):
+                conditionSet = self._readConditionElements(conditionSetElement)
+                if conditionSet is not None:
+                    ruleObject.conditionSets.append(conditionSet)
+            for subElement in ruleElement.findall('.sub'):
+                a = subElement.attrib['name']
+                b = subElement.attrib['with']
+                ruleObject.subs.append((a,b))
+            rules.append(ruleObject)
+        self.documentObject.rules = rules
+
+    def _readConditionElements(self, parentElement):
         cds = []
-                for conditionElement in conditionSetElement.findall('.condition'):
+        for conditionElement in parentElement.findall('.condition'):
             cd = {}
             cdMin = conditionElement.attrib.get("minimum")
             if cdMin is not None:
@@ -706,7 +703,7 @@ class BaseDocReader(object):
                 # will allow these to be None, assume axis.maximum
                 cd['maximum'] = None
             cd['name'] = conditionElement.attrib.get("name")
-                    # test for things
+            # # test for things
             if cd.get('minimum') is None and cd.get('maximum') is None:
                 if ruleObject.name is not None:
                     n = ruleObject.name
@@ -714,19 +711,12 @@ class BaseDocReader(object):
                     n = "%d" % len(rules)
                 raise DesignSpaceDocumentError("No minimum or maximum defined in rule \"%s\"." % n)
             cds.append(cd)
-                ruleObject.conditionSets.append(cds)
+        return cds
-            for subElement in ruleElement.findall('.sub'):
-                a = subElement.attrib['name']
-                b = subElement.attrib['with']
-                ruleObject.subs.append((a,b))
-            rules.append(ruleObject)
-        self.documentObject.rules = rules
 
     def readAxes(self):
         # read the axes elements, including the warp map.
         axes = []
         if len(self.root.findall(".axes/axis"))==0:
-            self.guessAxes()
             self._strictAxisNames = False
             return
         for axisElement in self.root.findall(".axes/axis"):
@@ -750,66 +740,7 @@ 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 = {}
-        for dimensionElement in locationElement.findall(".dimension"):
-            dimName = dimensionElement.attrib.get("name")
-            xValue = yValue = None
-            try:
-                xValue = dimensionElement.attrib.get('xvalue')
-                xValue = float(xValue)
-            except ValueError:
-                self.logger.info("KeyError in readLocation xValue %3.3f", xValue)
-            try:
-                yValue = dimensionElement.attrib.get('yvalue')
-                if yValue is not None:
-                    yValue = float(yValue)
-            except ValueError:
-                pass
-            if yValue is not None:
-                loc[dimName] = (xValue, yValue)
-            else:
-                loc[dimName] = xValue
-        return loc
-
-    def guessAxes(self):
-        # Called when we have no axes element in the file.
-        # Look at all locations and collect the axis names and values
-        # assumptions:
-        # look for the default value on an axis from a master location
-        # Needs deprecation warning
-        allLocations = []
-        minima = {}
-        maxima = {}
-        for locationElement in self.root.findall(".sources/source/location"):
-            allLocations.append(self._locationFromElement(locationElement))
-        for locationElement in self.root.findall(".instances/instance/location"):
-            allLocations.append(self._locationFromElement(locationElement))
-        for loc in allLocations:
-            for dimName, value in loc.items():
-                if not isinstance(value, tuple):
-                    value = [value]
-                for v in value:
-                    if dimName not in minima:
-                        minima[dimName] = v
-                        continue
-                    if minima[dimName] > v:
-                        minima[dimName] = v
-                    if dimName not in maxima:
-                        maxima[dimName] = v
-                        continue
-                    if maxima[dimName] < v:
-                        maxima[dimName] = v
-        newAxes = []
-        for axisName in maxima.keys():
-            a = self.axisDescriptorClass()
-            a.default = a.minimum = minima[axisName]
-            a.maximum = maxima[axisName]
-            a.name = axisName
-            a.tag, a.labelNames = tagForAxisName(axisName)
-            self.documentObject.axes.append(a)
 
     def readSources(self):
         for sourceCount, sourceElement in enumerate(self.root.findall(".sources/source")):
@@ -870,21 +801,21 @@ class BaseDocReader(object):
 
     def readLocationElement(self, locationElement):
         """ Format 0 location reader """
+        if not self.documentObject.axes:
+           raise DesignSpaceDocumentError("No axes defined.")
         loc = {}
         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.
+                self.logger.warning("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:
                 xValue = dimensionElement.attrib.get('xvalue')
                 xValue = float(xValue)
             except ValueError:
-                self.logger.info("KeyError in readLocation xValue %3.3f", xValue)
+                self.logger.warning("KeyError in readLocation xValue %3.3f", xValue)
             try:
                 yValue = dimensionElement.attrib.get('yvalue')
                 if yValue is not None:
@@ -963,49 +894,27 @@ class BaseDocReader(object):
         instanceObject.lib = from_plist(libElement[0])
 
     def readInfoElement(self, infoElement, instanceObject):
-        """ Read the info element.
+        """ Read the info element."""
-
-            ::
-
-                <info/>
-
-                Let's drop support for a different location for the info. Never needed it.
-
-            """
         infoLocation = self.locationFromElement(infoElement)
         instanceObject.info = True
 
     def readKerningElement(self, kerningElement, instanceObject):
-        """ Read the kerning element.
+        """ Read the kerning element."""
-
-        ::
-
-                Make kerning at the location and with the masters specified at the instance level.
-                <kerning/>
-
-        """
         kerningLocation = self.locationFromElement(kerningElement)
         instanceObject.addKerning(kerningLocation)
 
     def readGlyphElement(self, glyphElement, instanceObject):
         """
         Read the glyph element.
-
-        ::
-
             <glyph name="b" unicode="0x62"/>
-
             <glyph name="b"/>
-
             <glyph name="b">
                 <master location="location-token-bbb" source="master-token-aaa2"/>
                 <master glyphname="b.alt1" location="location-token-ccc" source="master-token-aaa3"/>
-
                 <note>
                     This is an instance from an anisotropic interpolation.
                 </note>
             </glyph>
-
         """
         glyphData = {}
         glyphName = glyphElement.attrib.get('name')
@@ -1057,7 +966,7 @@ class BaseDocReader(object):
 class DesignSpaceDocument(object):
     """ Read, write data from the designspace file"""
     def __init__(self, readerClass=None, writerClass=None):
-        self.logger = logging.getLogger("DesignSpaceDocumentLog")
+        self.logger = logging.getLogger("DesignSpaceLog")
         self.path = None
         self.filename = None
         """String, optional. When the document is read from the disk, this is
@@ -1094,6 +1003,8 @@ class DesignSpaceDocument(object):
         self.filename = os.path.basename(path)
         reader = self.readerClass(path, self)
         reader.read()
+        if self.sources:
+            self.findDefault()
 
     def write(self, path):
         self.path = path
@@ -1177,7 +1088,9 @@ class DesignSpaceDocument(object):
         self.rules.append(ruleDescriptor)
 
     def newDefaultLocation(self):
-        loc = {}
+        # Without OrderedDict, output XML would be non-deterministic.
+        # https://github.com/LettError/designSpaceDocument/issues/10
+        loc = collections.OrderedDict()
         for axisDescriptor in self.axes:
             loc[axisDescriptor.name] = axisDescriptor.default
         return loc
@@ -1222,151 +1135,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 sourcedescriptor with the location at all the defaults
-            This means making repairs if things are missing
+        # if we can't find it, return None, let someone else figure it out
-                - check if we have axes and deduce them from the masters if they're missing
+        self.default = None
-                - that can include axes referenced in masters, instances, glyphs.
-                - if no default is assigned, use mutatormath to find out.
-                - record the default in the designspace
-                - report all the changes in a log
-                - save a "repaired" version of the doc
-        """
-        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.location == self.defaultLoc:
-            if sourceDescriptor.copyInfo:
                 # we choose you!
-                flaggedDefaultCandidate = sourceDescriptor
+                self.default = sourceDescriptor
-        mutatorDefaultCandidate = self.getMutatorDefaultCandidate()
+                return sourceDescriptor
-        # what are we going to do?
-        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):
-        """
-            Make the axis data we have available in
-        """
-        benderAxes = {}
-        for axisDescriptor in self.axes:
-            d = {
-                'name': axisDescriptor.name,
-                'tag': axisDescriptor.tag,
-                'minimum': axisDescriptor.minimum,
-                'maximum': axisDescriptor.maximum,
-                'default': axisDescriptor.default,
-                'map': axisDescriptor.map,
-            }
-            benderAxes[axisDescriptor.name] = d
-        return benderAxes
-
-    def checkAxes(self, overwrite=False):
-        """
-            If we don't have axes in the document, make some, report
-            Should we include the instance locations when determining the axis extrema?
-        """
-        axisValues = {}
-        # find all the axes
-        locations = []
-        for sourceDescriptor in self.sources:
-            locations.append(sourceDescriptor.location)
-        for instanceDescriptor in self.instances:
-            locations.append(instanceDescriptor.location)
-            for name, glyphData in instanceDescriptor.glyphs.items():
-                loc = glyphData.get("instanceLocation")
-                if loc is not None:
-                    locations.append(loc)
-                for m in glyphData.get('masters', []):
-                    locations.append(m['location'])
-        for loc in locations:
-            for name, value in loc.items():
-                if not name in axisValues:
-                    axisValues[name] = []
-                if type(value)==tuple:
-                    for v in value:
-                        axisValues[name].append(v)
-                else:
-                    axisValues[name].append(value)
-        have = self.getAxisOrder()
-        for name, values in axisValues.items():
-            a = None
-            if name in have:
-                if overwrite:
-                    # we have the axis,
-                    a = self.getAxis(name)
-                else:
-                    continue
-            else:
-                # we need to make this axis
-                a = self.newAxisDescriptor()
-                self.addAxis(a)
-            a.name = name
-            a.minimum = min(values)
-            a.maximum = max(values)
-            a.default = a.minimum
-            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
+        # adapted from fontTools.varlib.models.normalizeLocation because:
-        # accept only values for the axes that we have definitions for
+        #   - this needs to work with axis names, not tags
-        # only normalise if we're valid?
+        #   - this needs to accomodate anisotropic locations
-        # normalise anisotropic cooordinates to isotropic.
+        #   - the axes are stored differently here, it's just math
-        # copied from fontTools.varlib.models.normalizeLocation
         new = {}
         for axis in self.axes:
             if not axis.name in location:
@@ -1391,6 +1176,7 @@ class DesignSpaceDocument(object):
         return new
 
     def normalize(self):
+        # Normalise the geometry of this designspace:
         #   scale all the locations of all masters and instances to the -1 - 0 - 1 value.
         #   we need the axis data to do the scaling, so we do those last.
         # masters
@@ -1404,7 +1190,7 @@ class DesignSpaceDocument(object):
                 for glyphMaster in glyphData['masters']:
                     glyphMaster['location'] = self.normalizeLocation(glyphMaster['location'])
             item.location = self.normalizeLocation(item.location)
-        # now the axes
+        # the axes
         for axis in self.axes:
             # scale the map first
             newMap = []
@@ -1439,20 +1225,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

@@ -5,12 +5,30 @@ from __future__ import (print_function, division, absolute_import,
 
 import os
 import pytest
+import warnings
 
 from fontTools.misc.py23 import open
 from fontTools.designspaceLib import (
     DesignSpaceDocument, SourceDescriptor, AxisDescriptor, RuleDescriptor,
     InstanceDescriptor, evaluateRule, processRules, posix, DesignSpaceDocumentError)
 
+def _axesAsDict(axes):
+    """
+        Make the axis data we have available in
+    """
+    axesDict = {}
+    for axisDescriptor in axes:
+        d = {
+            'name': axisDescriptor.name,
+            'tag': axisDescriptor.tag,
+            'minimum': axisDescriptor.minimum,
+            'maximum': axisDescriptor.maximum,
+            'default': axisDescriptor.default,
+            'map': axisDescriptor.map,
+        }
+        axesDict[axisDescriptor.name] = d
+    return axesDict
+
 
 def assert_equals_test_file(path, test_filename):
     with open(path) as fp:
@@ -31,6 +49,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 +148,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 +165,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 +187,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 +403,6 @@ def test_handleNoAxes(tmpdir):
     doc.addInstance(i1)
 
     doc.write(testDocPath)
-    __removeAxesFromDesignSpace(testDocPath)
     verify = DesignSpaceDocument()
     verify.read(testDocPath)
     verify.write(testDocPath2)
@@ -476,8 +421,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 +447,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 +464,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 +483,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 +499,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 +514,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
@@ -571,7 +529,8 @@ def test_pathNameResolve(tmpdir):
     assert doc.sources[0].filename == "masters/masterTest1.ufo"
 
 
-def test_normalise():
+def test_normalise1():
+    # normalisation of anisotropic locations, clipping
     doc = DesignSpaceDocument()
     # write some axes
     a1 = AxisDescriptor()
@@ -581,10 +540,8 @@ def test_normalise():
     a1.name = "axisName_a"
     a1.tag = "TAGA"
     doc.addAxis(a1)
-
     assert doc.normalizeLocation(dict(axisName_a=0)) == {'axisName_a': 0.0}
     assert doc.normalizeLocation(dict(axisName_a=1000)) == {'axisName_a': 1.0}
-
     # clipping beyond max values:
     assert doc.normalizeLocation(dict(axisName_a=1001)) == {'axisName_a': 1.0}
     assert doc.normalizeLocation(dict(axisName_a=500)) == {'axisName_a': 0.5}
@@ -599,6 +556,8 @@ def test_normalise():
     r.sort()
     assert r == [('axisName_a', -1.0, 0.0, 1.0)]
 
+def test_normalise2():
+    # normalisation with minimum > 0
     doc = DesignSpaceDocument()
     # write some axes
     a2 = AxisDescriptor()
@@ -624,6 +583,8 @@ def test_normalise():
     r.sort()
     assert r == [('axisName_b', 0.0, 0.0, 1.0)]
 
+def test_normalise3():
+    # normalisation of negative values, with default == maximum
     doc = DesignSpaceDocument()
     # write some axes
     a3 = AxisDescriptor()
@@ -636,7 +597,6 @@ def test_normalise():
     assert doc.normalizeLocation(dict(ccc=1)) == {'ccc': 0.0}
     assert doc.normalizeLocation(dict(ccc=-1000)) == {'ccc': -1.0}
     assert doc.normalizeLocation(dict(ccc=-1001)) == {'ccc': -1.0}
-
     doc.normalize()
     r = []
     for axis in doc.axes:
@@ -644,28 +604,8 @@ def test_normalise():
     r.sort()
     assert r == [('ccc', -1.0, 0.0, 0.0)]
 
-
+def test_normalise4():
-    doc = DesignSpaceDocument()
+    # normalisation with a map
-    # write some axes
-    a3 = AxisDescriptor()
-    a3.minimum = 2000
-    a3.maximum = 3000
-    a3.default = 2000
-    a3.name = "ccc"
-    doc.addAxis(a3)
-    assert doc.normalizeLocation(dict(ccc=0)) == {'ccc': 0.0}
-    assert doc.normalizeLocation(dict(ccc=1)) == {'ccc': 0.0}
-    assert doc.normalizeLocation(dict(ccc=-1000)) == {'ccc': 0.0}
-    assert doc.normalizeLocation(dict(ccc=-1001)) == {'ccc': 0.0}
-
-    doc.normalize()
-    r = []
-    for axis in doc.axes:
-        r.append((axis.name, axis.minimum, axis.default, axis.maximum))
-    r.sort()
-    assert r == [('ccc', 0.0, 0.0, 1.0)]
-
-
     doc = DesignSpaceDocument()
     # write some axes
     a4 = AxisDescriptor()
@@ -682,28 +622,27 @@ def test_normalise():
     r.sort()
     assert r == [('ddd', [(0, 0.1), (300, 0.5), (600, 0.5), (1000, 0.9)])]
 
-
+def test_axisMapping():
-def test_rules(tmpdir):
+    # note: because designspance lib does not do any actual
-    tmpdir = str(tmpdir)
+    # processing of the mapping data, we can only check if there data is there.
-    testDocPath = os.path.join(tmpdir, "testRules.designspace")
-    testDocPath2 = os.path.join(tmpdir, "testRules_roundtrip.designspace")
     doc = DesignSpaceDocument()
     # write some axes
-    a1 = AxisDescriptor()
+    a4 = AxisDescriptor()
-    a1.tag = "TAGA"
+    a4.minimum = 0
-    a1.name = "axisName_a"
+    a4.maximum = 1000
-    a1.minimum = 0
+    a4.default = 0
-    a1.maximum = 1000
+    a4.name = "ddd"
-    a1.default = 0
+    a4.map = [(0,100), (300, 500), (600, 500), (1000,900)]
-    doc.addAxis(a1)
+    doc.addAxis(a4)
-    a2 = AxisDescriptor()
+    doc.normalize()
-    a2.tag = "TAGB"
+    r = []
-    a2.name = "axisName_b"
+    for axis in doc.axes:
-    a2.minimum = 0
+        r.append((axis.name, axis.map))
-    a2.maximum = 3000
+    r.sort()
-    a2.default = 0
+    assert r == [('ddd', [(0, 0.1), (300, 0.5), (600, 0.5), (1000, 0.9)])]
-    doc.addAxis(a2)
 
+def test_rulesConditions(tmpdir):
+    # tests of rules, conditionsets and conditions
     r1 = RuleDescriptor()
     r1.name = "named.rule.1"
     r1.conditionSets.append([
@@ -712,11 +651,6 @@ def test_rules(tmpdir):
     ])
     r1.subs.append(("a", "a.alt"))
 
-    # rule with minium and maximum
-    doc.addRule(r1)
-    assert len(doc.rules) == 1
-    assert len(doc.rules[0].conditionSets) == 1
-    assert len(doc.rules[0].conditionSets[0]) == 2
     assert evaluateRule(r1, dict(axisName_a = 500, axisName_b = 0)) == True
     assert evaluateRule(r1, dict(axisName_a = 0, axisName_b = 0)) == True
     assert evaluateRule(r1, dict(axisName_a = 1000, axisName_b = 0)) == True
@@ -761,6 +695,12 @@ def test_rules(tmpdir):
     assert evaluateRule(r4, dict(axisName_a = 0, axisName_b = 0)) == False
     assert evaluateRule(r4, dict(axisName_a = 1000, axisName_b = 1000)) == False
 
+def test_rulesDocument(tmpdir):
+    # tests of rules in a document, roundtripping.
+    tmpdir = str(tmpdir)
+    testDocPath = os.path.join(tmpdir, "testRules.designspace")
+    testDocPath2 = os.path.join(tmpdir, "testRules_roundtrip.designspace")
+    doc = DesignSpaceDocument()
     a1 = AxisDescriptor()
     a1.minimum = 0
     a1.maximum = 1000
@@ -775,69 +715,77 @@ def test_rules(tmpdir):
     b1.tag = "TAGB"
     doc.addAxis(a1)
     doc.addAxis(b1)
-    assert doc._prepAxesForBender() == {'axisName_a': {'map': [], 'name': 'axisName_a', 'default': 0, 'minimum': 0, 'maximum': 1000, 'tag': 'TAGA'}, 'axisName_b': {'map': [], 'name': 'axisName_b', 'default': 2000, 'minimum': 2000, 'maximum': 3000, 'tag': 'TAGB'}}
+    r1 = RuleDescriptor()
-
+    r1.name = "named.rule.1"
+    r1.conditionSets.append([
+        dict(name='axisName_a', minimum=0, maximum=1000),
+        dict(name='axisName_b', minimum=0, maximum=3000)
+    ])
+    r1.subs.append(("a", "a.alt"))
+    # rule with minium and maximum
+    doc.addRule(r1)
+    assert len(doc.rules) == 1
+    assert len(doc.rules[0].conditionSets) == 1
+    assert len(doc.rules[0].conditionSets[0]) == 2
+    assert _axesAsDict(doc.axes) == {'axisName_a': {'map': [], 'name': 'axisName_a', 'default': 0, 'minimum': 0, 'maximum': 1000, 'tag': 'TAGA'}, 'axisName_b': {'map': [], 'name': 'axisName_b', 'default': 2000, 'minimum': 2000, 'maximum': 3000, 'tag': 'TAGB'}}
     assert doc.rules[0].conditionSets == [[
         {'minimum': 0, 'maximum': 1000, 'name': 'axisName_a'},
         {'minimum': 0, 'maximum': 3000, 'name': 'axisName_b'}]]
-
     assert doc.rules[0].subs == [('a', 'a.alt')]
-
     doc.normalize()
     assert doc.rules[0].name == 'named.rule.1'
     assert doc.rules[0].conditionSets == [[
         {'minimum': 0.0, 'maximum': 1.0, 'name': 'axisName_a'},
         {'minimum': 0.0, 'maximum': 1.0, 'name': 'axisName_b'}]]
-
+    # still one conditionset
+    assert len(doc.rules[0].conditionSets) == 1
     doc.write(testDocPath)
-    new = DesignSpaceDocument()
+    # add a stray conditionset
+    _addUnwrappedCondition(testDocPath)
+    doc2 = DesignSpaceDocument()
+    doc2.read(testDocPath)
+    assert len(doc2.axes) == 2
+    assert len(doc2.rules) == 1
+    assert len(doc2.rules[0].conditionSets) == 2
+    doc2.write(testDocPath2)
+    # verify these results
+    # make sure the stray condition is now neatly wrapped in a conditionset.
+    doc3 = DesignSpaceDocument()
+    doc3.read(testDocPath2)
+    assert len(doc3.rules) == 1
+    assert len(doc3.rules[0].conditionSets) == 2
 
-    new.read(testDocPath)
+def _addUnwrappedCondition(path):
-    assert len(new.axes) == 4
-    assert len(new.rules) == 1
-    new.write(testDocPath2)
-
-
-def test_incompleteRule(tmpdir):
-    tmpdir = str(tmpdir)
-    testDocPath1 = os.path.join(tmpdir, "testIncompleteRule.designspace")
-    doc = DesignSpaceDocument()
-    r1 = RuleDescriptor()
-    r1.name = "incomplete.rule.1"
-    r1.conditionSets.append([
-        dict(name='axisName_a', minimum=100),
-        dict(name='axisName_b', maximum=200)
-    ])
-    r1.subs.append(("c", "c.alt"))
-    doc.addRule(r1)
-    doc.write(testDocPath1)
-    __removeConditionMinimumMaximumDesignSpace(testDocPath1)
-    new = DesignSpaceDocument()
-    with pytest.raises(DesignSpaceDocumentError) as excinfo:
-        new.read(testDocPath1)
-    assert "No minimum or maximum defined in rule" in str(excinfo.value)
-
-def __removeConditionMinimumMaximumDesignSpace(path):
     # only for testing, so we can make an invalid designspace file
-    # without making the designSpaceDocument also support it.
+    # older designspace files may have conditions that are not wrapped in a conditionset
+    # These can be read into a new conditionset.
     f = open(path, 'r', encoding='utf-8')
     d = f.read()
+    print(d)
     f.close()
-    d = d.replace(' minimum="100"', '')
+    d = d.replace('<rule name="named.rule.1">', '<rule name="named.rule.1">\n\t<condition maximum="22" minimum="33" name="axisName_a" />')
     f = open(path, 'w', encoding='utf-8')
     f.write(d)
     f.close()
 
-def __removeAxesFromDesignSpace(path):
+def test_documentLib(tmpdir):
-    # only for testing, so we can make an invalid designspace file
+    # roundtrip test of the document lib with some nested data
-    # without making the designSpaceDocument also support it.
+    tmpdir = str(tmpdir)
-    f = open(path, 'r', encoding='utf-8')
+    testDocPath1 = os.path.join(tmpdir, "testDocumentLibTest.designspace")
-    d = f.read()
+    doc = DesignSpaceDocument()
-    f.close()
+    a1 = AxisDescriptor()
-    start = d.find("<axes>")
+    a1.tag = "TAGA"
-    end = d.find("</axes>")+len("</axes>")
+    a1.name = "axisName_a"
-    n = d[0:start] + d[end:]
+    a1.minimum = 0
-    f = open(path, 'w', encoding='utf-8')
+    a1.maximum = 1000
-    f.write(n)
+    a1.default = 0
-    f.close()
+    doc.addAxis(a1)
+    dummyData = dict(a=123, b=u"äbc", c=[1,2,3], d={'a':123})
+    dummyKey = "org.fontTools.designspaceLib"
+    doc.lib = {dummyKey: dummyData}
+    doc.write(testDocPath1)
+    new = DesignSpaceDocument()
+    new.read(testDocPath1)
+    assert dummyKey in new.lib
+    assert new.lib[dummyKey] == dummyData