From 7fc2e37e419d299e152314af77263fe91707703f Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 12:00:31 +0000 Subject: [PATCH 01/20] Try harder to get a name from file being merged --- Lib/fontTools/varLib/errors.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index 128629bfc..b5680ec18 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -32,11 +32,18 @@ class VarLibMergeError(VarLibError): self.merger = merger self.args = args + def _master_name(self, ttf, ix): + if "name" in ttf: + return ttf["name"].getDebugName(1) + " " + ttf["name"].getDebugName(2) + elif hasattr(ttf.reader, "file") and hasattr(ttf.reader.file, "name"): + return ttf.reader.file.name + else: + return "master number %i" % ix + def __str__(self): cause, stack = self.args[0], self.args[1:] fontnames = [ - ttf["name"].getDebugName(1) + " " + ttf["name"].getDebugName(2) - for ttf in self.merger.ttfs + self._master_name(ttf, ix) for ix, ttf in enumerate(self.merger.ttfs) ] context = "".join(reversed(stack)) details = "" From e2a859d649131fa77a53fc2ecb4094c667c1bbdc Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 12:04:36 +0000 Subject: [PATCH 02/20] Ensure table name is in error message stack --- Lib/fontTools/varLib/merger.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index 969a7a05b..0ea6c2cf6 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -123,8 +123,12 @@ class Merger(object): self.ttfs = master_ttfs # For error reporting for tag in tableTags: if tag not in font: continue - self.mergeThings(font[tag], [m[tag] if tag in m else None - for m in master_ttfs]) + try: + self.mergeThings(font[tag], [m[tag] if tag in m else None + for m in master_ttfs]) + except Exception as e: + e.args = e.args + (tag,) + raise # # Aligning merger From 5bac84374b267e9a096b1d1baf39a03f210cee6e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 12:50:07 +0000 Subject: [PATCH 03/20] Ensure TTF list is correct --- Lib/fontTools/varLib/merger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index 0ea6c2cf6..6b0416b29 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -120,10 +120,10 @@ class Merger(object): "got": lst},)) def mergeTables(self, font, master_ttfs, tableTags): - self.ttfs = master_ttfs # For error reporting for tag in tableTags: if tag not in font: continue try: + self.ttfs = [m for m in master_ttfs if tag in m] self.mergeThings(font[tag], [m[tag] if tag in m else None for m in master_ttfs]) except Exception as e: From 46bd7a7e974220f1e99c937c79112ace97df29dd Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 12:50:47 +0000 Subject: [PATCH 04/20] Many fixes... --- Lib/fontTools/varLib/errors.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index b5680ec18..35cc6a1e8 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -32,7 +32,8 @@ class VarLibMergeError(VarLibError): self.merger = merger self.args = args - def _master_name(self, ttf, ix): + def _master_name(self, ix): + ttf = self.merger.ttfs[ix] if "name" in ttf: return ttf["name"].getDebugName(1) + " " + ttf["name"].getDebugName(2) elif hasattr(ttf.reader, "file") and hasattr(ttf.reader.file, "name"): @@ -40,31 +41,36 @@ class VarLibMergeError(VarLibError): else: return "master number %i" % ix + def _offender(self, cause): + reason = cause["reason"].value + if "expected" in cause and "got" in cause: + index = [x == cause["expected"] for x in cause["got"]].index(False) + return index, self._master_name(index) + if reason == VarLibMergeFailure.FoundANone: + index = [x is None for x in cause["got"]].index(True) + return index, self._master_name(index) + return None, None + def __str__(self): cause, stack = self.args[0], self.args[1:] - fontnames = [ - self._master_name(ttf, ix) for ix, ttf in enumerate(self.merger.ttfs) - ] context = "".join(reversed(stack)) details = "" reason = cause["reason"].value - if reason == VarLibMergeFailure.FoundANone: - offender = [x is None for x in cause["got"]].index(True) - details = ( - f"\n\nThe problem is likely to be in {fontnames[offender]}:\n" - f"{stack[0]}=={cause['got']}\n" - ) + offender_index, offender = self._offender(cause) + if offender: + details = f"\n\nThe problem is likely to be in {offender}:\n" + if cause["reason"] == VarLibMergeFailure.FoundANone: + details = details + f"{stack[0]}=={cause['got']}\n" elif "expected" in cause and "got" in cause: offender = [x == cause["expected"] for x in cause["got"]].index(False) got = cause["got"][offender] - details = ( - f"\n\nThe problem is likely to be in {fontnames[offender]}:\n" + details = details + ( f"Expected to see {stack[0]}=={cause['expected']}, instead saw {got}\n" ) if ( - reason == VarLibMergeFailure.UnsupportedFormat - or reason == VarLibMergeFailure.InconsistentFormat + cause["reason"] == VarLibMergeFailure.UnsupportedFormat + or cause["reason"] == VarLibMergeFailure.InconsistentFormat ): reason = reason % cause["subtable"] basic = textwrap.fill( From 1bf3ccceadbc7c256004fd96683c78726b7eab66 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 12:51:02 +0000 Subject: [PATCH 05/20] Provide additional information for a common failure --- Lib/fontTools/varLib/errors.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index 35cc6a1e8..e06e29479 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -51,6 +51,23 @@ class VarLibMergeError(VarLibError): return index, self._master_name(index) return None, None + def _incompatible_features(self, offender_index): + cause, stack = self.args[0], self.args[1:] + bad_ttf = self.merger.ttfs[offender_index] + good_ttf = self.merger.ttfs[offender_index - 1] + + good_features = [ + x.FeatureTag for x in good_ttf[stack[-1]].table.FeatureList.FeatureRecord + ] + bad_features = [ + x.FeatureTag for x in bad_ttf[stack[-1]].table.FeatureList.FeatureRecord + ] + return ( + "\nIncompatible features between masters.\n" + f"Expected: {', '.join(good_features)}.\n" + f"Got: {', '.join(bad_features)}.\n" + ) + def __str__(self): cause, stack = self.args[0], self.args[1:] context = "".join(reversed(stack)) @@ -61,6 +78,9 @@ class VarLibMergeError(VarLibError): details = f"\n\nThe problem is likely to be in {offender}:\n" if cause["reason"] == VarLibMergeFailure.FoundANone: details = details + f"{stack[0]}=={cause['got']}\n" + elif cause["reason"] == VarLibMergeFailure.ShouldBeConstant: + # Common case + details = details + self._incompatible_features(offender_index) elif "expected" in cause and "got" in cause: offender = [x == cause["expected"] for x in cause["got"]].index(False) got = cause["got"][offender] From 0a1aa19c390bad00cd513961744e2724fc4509f5 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Mon, 15 Mar 2021 13:12:11 +0000 Subject: [PATCH 06/20] Test for better error message --- .../data/IncompatibleFeatures.designspace | 22 + .../IncompatibleFeatures-Bold.ttx | 578 ++++++++++++++++ .../IncompatibleFeatures-Regular.ttx | 626 ++++++++++++++++++ Tests/varLib/varLib_test.py | 27 +- 4 files changed, 1252 insertions(+), 1 deletion(-) create mode 100644 Tests/varLib/data/IncompatibleFeatures.designspace create mode 100644 Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Bold.ttx create mode 100644 Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Regular.ttx diff --git a/Tests/varLib/data/IncompatibleFeatures.designspace b/Tests/varLib/data/IncompatibleFeatures.designspace new file mode 100644 index 000000000..ab275164a --- /dev/null +++ b/Tests/varLib/data/IncompatibleFeatures.designspace @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Bold.ttx b/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Bold.ttx new file mode 100644 index 000000000..a1803e9e9 --- /dev/null +++ b/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Bold.ttx @@ -0,0 +1,578 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Bold + + + 1.000;NONE;SimpleTwoAxis-Bold + + + Simple Two Axis Bold + + + Version 1.000 + + + SimpleTwoAxis-Bold + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Regular.ttx b/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Regular.ttx new file mode 100644 index 000000000..7186a3e97 --- /dev/null +++ b/Tests/varLib/data/master_incompatible_features/IncompatibleFeatures-Regular.ttx @@ -0,0 +1,626 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Regular + + + 1.000;NONE;SimpleTwoAxis-Regular + + + Simple Two Axis Regular + + + Version 1.000 + + + SimpleTwoAxis-Regular + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index da1f94f4a..76d951def 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -1,7 +1,7 @@ from fontTools.misc.py23 import * from fontTools.ttLib import TTFont, newTable from fontTools.varLib import build, load_designspace -from fontTools.varLib.errors import VarLibValidationError +from fontTools.varLib.errors import VarLibValidationError, VarLibMergeError from fontTools.varLib.mutator import instantiateVariableFont from fontTools.varLib import main as varLib_main, load_masters from fontTools.varLib import set_default_weight_width_slant @@ -813,7 +813,32 @@ class BuildTest(unittest.TestCase): assert ds_loaded.instances[0].location == {"weight": 0, "width": 50} + def test_varlib_build_incompatible_features(self): + try: + self._run_varlib_build_test( + designspace_name="IncompatibleFeatures", + font_name="IncompatibleFeatures", + tables=["GPOS"], + expected_ttx_name="IncompatibleFeatures", + save_before_dump=True, + ) + except VarLibMergeError as e: + assert str(e) == """ +Couldn't merge the fonts, because some values were different, but should have +been the same. This happened while performing the following operation: +GPOS.table.FeatureList.FeatureCount + +The problem is likely to be in Simple Two Axis Bold: + +Incompatible features between masters. +Expected: kern, mark. +Got: kern. +""" + except Exception: + self.fail('unexpected exception raised') + else: + self.fail('ExpectedException not raised') def test_load_masters_layerName_without_required_font(): ds = DesignSpaceDocument() s = SourceDescriptor() From 939962f8580ed7e5468085db3d8d958a878ded34 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 11:52:11 +0000 Subject: [PATCH 07/20] Check we can actually get a name --- Lib/fontTools/varLib/errors.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index e06e29479..aa70b4786 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -34,7 +34,11 @@ class VarLibMergeError(VarLibError): def _master_name(self, ix): ttf = self.merger.ttfs[ix] - if "name" in ttf: + if ( + "name" in ttf + and ttf["name"].getDebugName(1) + and ttf["name"].getDebugName(2) + ): return ttf["name"].getDebugName(1) + " " + ttf["name"].getDebugName(2) elif hasattr(ttf.reader, "file") and hasattr(ttf.reader.file, "name"): return ttf.reader.file.name From 539b3cd71dc807bf678cec524a8de9ae1d1e5dd5 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 11:52:50 +0000 Subject: [PATCH 08/20] Special case must actually be special-cased --- Lib/fontTools/varLib/errors.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index aa70b4786..0c11a4e50 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -82,7 +82,10 @@ class VarLibMergeError(VarLibError): details = f"\n\nThe problem is likely to be in {offender}:\n" if cause["reason"] == VarLibMergeFailure.FoundANone: details = details + f"{stack[0]}=={cause['got']}\n" - elif cause["reason"] == VarLibMergeFailure.ShouldBeConstant: + elif ( + cause["reason"] == VarLibMergeFailure.ShouldBeConstant + and stack[0] == ".FeatureCount" + ): # Common case details = details + self._incompatible_features(offender_index) elif "expected" in cause and "got" in cause: From 8a6e3087ce38fa09751930d1a6cca1a40e875c6c Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 11:55:51 +0000 Subject: [PATCH 09/20] Use pytest exception handling --- Tests/varLib/varLib_test.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 76d951def..2a4cff767 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -814,16 +814,9 @@ class BuildTest(unittest.TestCase): assert ds_loaded.instances[0].location == {"weight": 0, "width": 50} def test_varlib_build_incompatible_features(self): - try: - self._run_varlib_build_test( - designspace_name="IncompatibleFeatures", - font_name="IncompatibleFeatures", - tables=["GPOS"], - expected_ttx_name="IncompatibleFeatures", - save_before_dump=True, - ) - except VarLibMergeError as e: - assert str(e) == """ + with pytest.raises( + VarLibMergeError, + match = """ Couldn't merge the fonts, because some values were different, but should have been the same. This happened while performing the following operation: @@ -834,11 +827,16 @@ The problem is likely to be in Simple Two Axis Bold: Incompatible features between masters. Expected: kern, mark. Got: kern. -""" - except Exception: - self.fail('unexpected exception raised') - else: - self.fail('ExpectedException not raised') +"""): + + self._run_varlib_build_test( + designspace_name="IncompatibleFeatures", + font_name="IncompatibleFeatures", + tables=["GPOS"], + expected_ttx_name="IncompatibleFeatures", + save_before_dump=True, + ) + def test_load_masters_layerName_without_required_font(): ds = DesignSpaceDocument() s = SourceDescriptor() From b26728d3cbe6501a1611cddc4e015c1fcb92f38e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 11:56:02 +0000 Subject: [PATCH 10/20] Specifically catch VarLibMergeError --- Lib/fontTools/varLib/merger.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index 6b0416b29..a48759392 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -82,7 +82,7 @@ class Merger(object): values = [getattr(table, key) for table in lst] mergerFunc = mergers.get(key, defaultMerger) mergerFunc(self, value, values) - except Exception as e: + except VarLibMergeError as e: e.args = e.args + ('.'+key,) raise @@ -95,7 +95,7 @@ class Merger(object): for i,(value,values) in enumerate(zip(out, zip(*lst))): try: self.mergeThings(value, values) - except Exception as e: + except VarLibMergeError as e: e.args = e.args + ('[%d]' % i,) raise @@ -126,7 +126,7 @@ class Merger(object): self.ttfs = [m for m in master_ttfs if tag in m] self.mergeThings(font[tag], [m[tag] if tag in m else None for m in master_ttfs]) - except Exception as e: + except VarLibMergeError as e: e.args = e.args + (tag,) raise From bfc4ac95245ce49ff8b1eac80366359bfcc97a68 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 14:59:11 +0000 Subject: [PATCH 11/20] Compare enums by identity --- Lib/fontTools/varLib/errors.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index 0c11a4e50..d99f70ab8 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -50,7 +50,7 @@ class VarLibMergeError(VarLibError): if "expected" in cause and "got" in cause: index = [x == cause["expected"] for x in cause["got"]].index(False) return index, self._master_name(index) - if reason == VarLibMergeFailure.FoundANone: + if reason is VarLibMergeFailure.FoundANone: index = [x is None for x in cause["got"]].index(True) return index, self._master_name(index) return None, None @@ -80,10 +80,10 @@ class VarLibMergeError(VarLibError): offender_index, offender = self._offender(cause) if offender: details = f"\n\nThe problem is likely to be in {offender}:\n" - if cause["reason"] == VarLibMergeFailure.FoundANone: + if cause["reason"] is VarLibMergeFailure.FoundANone: details = details + f"{stack[0]}=={cause['got']}\n" elif ( - cause["reason"] == VarLibMergeFailure.ShouldBeConstant + cause["reason"] is VarLibMergeFailure.ShouldBeConstant and stack[0] == ".FeatureCount" ): # Common case @@ -96,8 +96,8 @@ class VarLibMergeError(VarLibError): ) if ( - cause["reason"] == VarLibMergeFailure.UnsupportedFormat - or cause["reason"] == VarLibMergeFailure.InconsistentFormat + cause["reason"] is VarLibMergeFailure.UnsupportedFormat + or cause["reason"] is VarLibMergeFailure.InconsistentFormat ): reason = reason % cause["subtable"] basic = textwrap.fill( From 02ebcf8077b8208300da60986576902bd71bc21e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 17 Mar 2021 14:59:36 +0000 Subject: [PATCH 12/20] Redundant computation --- Lib/fontTools/varLib/errors.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index d99f70ab8..390f48218 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -89,8 +89,7 @@ class VarLibMergeError(VarLibError): # Common case details = details + self._incompatible_features(offender_index) elif "expected" in cause and "got" in cause: - offender = [x == cause["expected"] for x in cause["got"]].index(False) - got = cause["got"][offender] + got = cause["got"][offender_index] details = details + ( f"Expected to see {stack[0]}=={cause['expected']}, instead saw {got}\n" ) From 6c547864b615afd9c4fcb045122441128203563e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 18 Mar 2021 15:49:49 +0000 Subject: [PATCH 13/20] Use individual exception classes instead of enum --- Lib/fontTools/varLib/errors.py | 169 +++++++++++++++++++++------------ Lib/fontTools/varLib/merger.py | 80 +++++++--------- 2 files changed, 144 insertions(+), 105 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index 390f48218..ca502dacb 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -1,22 +1,6 @@ -from enum import Enum, auto import textwrap -class VarLibMergeFailure(Enum): - ShouldBeConstant = "some values were different, but should have been the same" - MismatchedTypes = "data had inconsistent types" - LengthsDiffer = "a list of objects had inconsistent lengths" - KeysDiffer = "a list of objects had different keys" - InconsistentGlyphOrder = "the glyph order was inconsistent between masters" - FoundANone = "one of the values in a list was empty when it shouldn't have been" - NotANone = "one of the values in a list was not empty when it should have been" - UnsupportedFormat = "an OpenType subtable (%s) had a format I didn't expect" - InconsistentFormat = ( - "an OpenType subtable (%s) had inconsistent formats between masters" - ) - InconsistentExtensions = "the masters use extension lookups in inconsistent ways" - - class VarLibError(Exception): """Base exception for the varLib module.""" @@ -32,6 +16,18 @@ class VarLibMergeError(VarLibError): self.merger = merger self.args = args + @property + def cause(self): + return self.args[0] + + @property + def stack(self): + return self.args[1:] + + @property + def reason(self): + return self.__doc__ + def _master_name(self, ix): ttf = self.merger.ttfs[ix] if ( @@ -45,26 +41,55 @@ class VarLibMergeError(VarLibError): else: return "master number %i" % ix - def _offender(self, cause): - reason = cause["reason"].value - if "expected" in cause and "got" in cause: - index = [x == cause["expected"] for x in cause["got"]].index(False) - return index, self._master_name(index) - if reason is VarLibMergeFailure.FoundANone: - index = [x is None for x in cause["got"]].index(True) + @property + def offender(self): + if "expected" in self.cause and "got" in self.cause: + index = [x == self.cause["expected"] for x in self.cause["got"]].index( + False + ) return index, self._master_name(index) return None, None - def _incompatible_features(self, offender_index): - cause, stack = self.args[0], self.args[1:] + @property + def details(self): + if "expected" in self.cause and "got" in self.cause: + offender_index, offender = self.offender + got = self.cause["got"][offender_index] + return f"Expected to see {self.stack[0]}=={self.cause['expected']}, instead saw {got}\n" + return "" + + def __str__(self): + offender_index, offender = self.offender + location = "" + if offender: + location = f"\n\nThe problem is likely to be in {offender}:\n" + context = "".join(reversed(self.stack)) + basic = textwrap.fill( + f"Couldn't merge the fonts, because {self.reason}. " + f"This happened while performing the following operation: {context}", + width=78, + ) + return "\n\n" + basic + location + self.details + + +class VarLibMergeFailureShouldBeConstant(VarLibMergeError): + """some values were different, but should have been the same""" + + @property + def details(self): + if self.stack[0] != ".FeatureCount": + return super() + offender_index, offender = self.offender bad_ttf = self.merger.ttfs[offender_index] good_ttf = self.merger.ttfs[offender_index - 1] good_features = [ - x.FeatureTag for x in good_ttf[stack[-1]].table.FeatureList.FeatureRecord + x.FeatureTag + for x in good_ttf[self.stack[-1]].table.FeatureList.FeatureRecord ] bad_features = [ - x.FeatureTag for x in bad_ttf[stack[-1]].table.FeatureList.FeatureRecord + x.FeatureTag + for x in bad_ttf[self.stack[-1]].table.FeatureList.FeatureRecord ] return ( "\nIncompatible features between masters.\n" @@ -72,39 +97,65 @@ class VarLibMergeError(VarLibError): f"Got: {', '.join(bad_features)}.\n" ) - def __str__(self): - cause, stack = self.args[0], self.args[1:] - context = "".join(reversed(stack)) - details = "" - reason = cause["reason"].value - offender_index, offender = self._offender(cause) - if offender: - details = f"\n\nThe problem is likely to be in {offender}:\n" - if cause["reason"] is VarLibMergeFailure.FoundANone: - details = details + f"{stack[0]}=={cause['got']}\n" - elif ( - cause["reason"] is VarLibMergeFailure.ShouldBeConstant - and stack[0] == ".FeatureCount" - ): - # Common case - details = details + self._incompatible_features(offender_index) - elif "expected" in cause and "got" in cause: - got = cause["got"][offender_index] - details = details + ( - f"Expected to see {stack[0]}=={cause['expected']}, instead saw {got}\n" - ) - if ( - cause["reason"] is VarLibMergeFailure.UnsupportedFormat - or cause["reason"] is VarLibMergeFailure.InconsistentFormat - ): - reason = reason % cause["subtable"] - basic = textwrap.fill( - f"Couldn't merge the fonts, because {reason}. " - f"This happened while performing the following operation: {context}", - width=78, - ) - return "\n\n" + basic + details +class VarLibMergeFailureFoundANone(VarLibMergeError): + """one of the values in a list was empty when it shouldn't have been""" + + @property + def offender(self): + cause = self.argv[0] + index = [x is None for x in cause["got"]].index(True) + return index, self._master_name(index) + + @property + def details(self): + cause, stack = self.args[0], self.args[1:] + return f"{stack[0]}=={cause['got']}\n" + + +class VarLibMergeFailureMismatchedTypes(VarLibMergeError): + """data had inconsistent types""" + + pass + + +class VarLibMergeFailureLengthsDiffer(VarLibMergeError): + """a list of objects had inconsistent lengths""" + + pass + + +class VarLibMergeFailureKeysDiffer(VarLibMergeError): + """a list of objects had different keys""" + + pass + + +class VarLibMergeFailureInconsistentGlyphOrder(VarLibMergeError): + """the glyph order was inconsistent between masters""" + + pass + + +class VarLibMergeFailureInconsistentExtensions(VarLibMergeError): + """the masters use extension lookups in inconsistent ways""" + + pass + + +class VarLibMergeFailureUnsupportedFormat(VarLibMergeError): + """an OpenType subtable (%s) had a format I didn't expect""" + + @property + def reason(self): + cause, stack = self.args[0], self.args[1:] + return self.__doc__ % cause["subtable"] + + +class VarLibMergeFailureUnsupportedFormat(VarLibMergeFailureUnsupportedFormat): + """an OpenType subtable (%s) had inconsistent formats between masters""" + + pass class VarLibCFFDictMergeError(VarLibMergeError): diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index a48759392..14910ad2f 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -14,8 +14,18 @@ from fontTools.varLib.varStore import VarStoreInstancer from functools import reduce from fontTools.otlLib.builder import buildSinglePos -from .errors import VarLibMergeError, VarLibMergeFailure - +from .errors import ( + VarLibMergeFailureShouldBeConstant, + VarLibMergeFailureFoundANone, + VarLibMergeFailureMismatchedTypes, + VarLibMergeFailureLengthsDiffer, + VarLibMergeFailureKeysDiffer, + VarLibMergeFailureInconsistentGlyphOrder, + VarLibMergeFailureInconsistentExtensions, + VarLibMergeFailureUnsupportedFormat, + VarLibMergeFailureUnsupportedFormat, + VarLibMergeError, +) class Merger(object): @@ -69,8 +79,7 @@ class Merger(object): item.ensureDecompiled() keys = sorted(vars(out).keys()) if not all(keys == sorted(vars(v).keys()) for v in lst): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.KeysDiffer, + raise VarLibMergeFailureKeysDiffer(self, ({ "expected": keys, "got": [sorted(vars(v).keys()) for v in lst]},)) mergers = self.mergersFor(out) @@ -88,8 +97,7 @@ class Merger(object): def mergeLists(self, out, lst): if not allEqualTo(out, lst, len): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.LengthsDiffer, + raise VarLibMergeFailureLengthsDiffer(self, ({ "expected": len(out), "got": [len(x) for x in lst]},)) for i,(value,values) in enumerate(zip(out, zip(*lst))): @@ -101,8 +109,7 @@ class Merger(object): def mergeThings(self, out, lst): if not allEqualTo(out, lst, type): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.MismatchedTypes, + raise VarLibMergeFailureMismatchedTypes(self, ({ "expected": type(out), "got": [type(x) for x in lst]},)) mergerFunc = self.mergersFor(out).get(None, None) @@ -114,8 +121,7 @@ class Merger(object): self.mergeLists(out, lst) else: if not allEqualTo(out, lst): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.ShouldBeConstant, + raise VarLibMergeFailureShouldBeConstant(self, ({ "expected": out, "got": lst},)) @@ -140,8 +146,7 @@ class AligningMerger(Merger): def merge(merger, self, lst): if self is None: if not allNone(lst): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.NotANone, + raise VarLibMergeFailureNotANone(self, ({ "expected": None, "got": lst },)) @@ -157,8 +162,7 @@ def merge(merger, self, lst): for k in allKeys: allValues = nonNone(l.get(k) for l in lst) if not allEqual(allValues): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.ShouldBeConstant, + raise VarLibMergeFailureShouldBeConstant(self, ({ "expected": allValues[0], "got": lst, }, "."+k)) @@ -198,8 +202,7 @@ def _merge_GlyphOrders(font, lst, values_lst=None, default=None): order = sorted(combined, key=sortKey) # Make sure all input glyphsets were in proper order if not all(sorted(vs, key=sortKey) == vs for vs in lst): - raise VarLibMergeError(self, ({ - "reason" : VarLibMergeFailure.InconsistentGlyphOrder + raise VarLibMergeFailureInconsistentGlyphOrder(self, ({ },)) del combined @@ -227,8 +230,7 @@ def _Lookup_SinglePos_get_effective_value(subtables, glyph): elif self.Format == 2: return self.Value[self.Coverage.glyphs.index(glyph)] else: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "single positioning lookup" },)) return None @@ -252,8 +254,7 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0) return self.Class1Record[klass1].Class2Record[klass2] else: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "pair positioning lookup" },)) return None @@ -262,8 +263,7 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) def merge(merger, self, lst): self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0) if not (len(lst) == 1 or (valueFormat & ~0xF == 0)): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "single positioning lookup" },)) @@ -552,8 +552,7 @@ def merge(merger, self, lst): elif self.Format == 2: _PairPosFormat2_merge(self, lst, merger) else: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "pair positioning lookup" },)) @@ -621,8 +620,7 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): # input masters. if not allEqual(allClasses): - raise VarLibMergeError(self, allClasses) - if not allClasses: + raise allClasses(self, allClasses) rec = None else: rec = ot.MarkRecord() @@ -671,32 +669,28 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): @AligningMerger.merger(ot.MarkBasePos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.InconsistentFormats, + raise VarLibMergeFailureInconsistentFormats(self, ({ "subtable": "mark-to-base positioning lookup", "expected": self.Format, "got": [l.Format for l in lst]},)) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger) else: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "mark-to-base positioning lookup", })) @AligningMerger.merger(ot.MarkMarkPos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.InconsistentFormats, + raise VarLibMergeFailureInconsistentFormats(self, ({ "subtable": "mark-to-mark positioning lookup", "expected": self.Format, "got": [l.Format for l in lst]},)) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2') else: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "mark-to-mark positioning lookup", })) @@ -828,14 +822,12 @@ def merge(merger, self, lst): continue if sts[0].__class__.__name__.startswith('Extension'): if not allEqual([st.__class__ for st in sts]): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.InconsistentExtensions, + raise VarLibMergeFailureInconsistentExtensions(self, ({ "expected": "Extension", "got": [st.__class__.__name__ for st in sts] },)) if not allEqual([st.ExtensionLookupType for st in sts]): - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.InconsistentExtensions, + raise VarLibMergeFailureInconsistentExtensions(self, ({ },)) l.LookupType = sts[0].ExtensionLookupType new_sts = [st.ExtSubTable for st in sts] @@ -1065,8 +1057,7 @@ class VariationMerger(AligningMerger): if None in lst: if allNone(lst): if out is not None: - raise VarLibMergeError(self, ({ - "reason": VarLibMergeFailure.FoundANone, + raise VarLibMergeFailureFoundANone(self, ({ "got": lst },)) return @@ -1089,8 +1080,7 @@ def buildVarDevTable(store_builder, master_values): @VariationMerger.merger(ot.BaseCoord) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeError(self, ({ - "cause": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "a baseline coordinate" },)) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) @@ -1101,8 +1091,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.CaretValue) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeError(self, ({ - "cause": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "a caret" },)) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) @@ -1113,8 +1102,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.Anchor) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeError(self, ({ - "cause": VarLibMergeFailure.UnsupportedFormat, + raise VarLibMergeFailureUnsupportedFormat(self, ({ "subtable": "an anchor" },)) self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst]) From 3e0a87a14685a3c4d54081481b1e4de5c8ba2f5e Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 18 Mar 2021 20:58:11 +0000 Subject: [PATCH 14/20] Shorten exception names --- Lib/fontTools/varLib/errors.py | 18 +++++----- Lib/fontTools/varLib/merger.py | 60 +++++++++++++++++----------------- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index ca502dacb..daaaef602 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -72,7 +72,7 @@ class VarLibMergeError(VarLibError): return "\n\n" + basic + location + self.details -class VarLibMergeFailureShouldBeConstant(VarLibMergeError): +class ShouldBeConstant(VarLibMergeError): """some values were different, but should have been the same""" @property @@ -98,7 +98,7 @@ class VarLibMergeFailureShouldBeConstant(VarLibMergeError): ) -class VarLibMergeFailureFoundANone(VarLibMergeError): +class FoundANone(VarLibMergeError): """one of the values in a list was empty when it shouldn't have been""" @property @@ -113,37 +113,37 @@ class VarLibMergeFailureFoundANone(VarLibMergeError): return f"{stack[0]}=={cause['got']}\n" -class VarLibMergeFailureMismatchedTypes(VarLibMergeError): +class MismatchedTypes(VarLibMergeError): """data had inconsistent types""" pass -class VarLibMergeFailureLengthsDiffer(VarLibMergeError): +class LengthsDiffer(VarLibMergeError): """a list of objects had inconsistent lengths""" pass -class VarLibMergeFailureKeysDiffer(VarLibMergeError): +class KeysDiffer(VarLibMergeError): """a list of objects had different keys""" pass -class VarLibMergeFailureInconsistentGlyphOrder(VarLibMergeError): +class InconsistentGlyphOrder(VarLibMergeError): """the glyph order was inconsistent between masters""" pass -class VarLibMergeFailureInconsistentExtensions(VarLibMergeError): +class InconsistentExtensions(VarLibMergeError): """the masters use extension lookups in inconsistent ways""" pass -class VarLibMergeFailureUnsupportedFormat(VarLibMergeError): +class UnsupportedFormat(VarLibMergeError): """an OpenType subtable (%s) had a format I didn't expect""" @property @@ -152,7 +152,7 @@ class VarLibMergeFailureUnsupportedFormat(VarLibMergeError): return self.__doc__ % cause["subtable"] -class VarLibMergeFailureUnsupportedFormat(VarLibMergeFailureUnsupportedFormat): +class UnsupportedFormat(UnsupportedFormat): """an OpenType subtable (%s) had inconsistent formats between masters""" pass diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index 14910ad2f..ffe4cf8b1 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -15,15 +15,15 @@ from functools import reduce from fontTools.otlLib.builder import buildSinglePos from .errors import ( - VarLibMergeFailureShouldBeConstant, - VarLibMergeFailureFoundANone, - VarLibMergeFailureMismatchedTypes, - VarLibMergeFailureLengthsDiffer, - VarLibMergeFailureKeysDiffer, - VarLibMergeFailureInconsistentGlyphOrder, - VarLibMergeFailureInconsistentExtensions, - VarLibMergeFailureUnsupportedFormat, - VarLibMergeFailureUnsupportedFormat, + ShouldBeConstant, + FoundANone, + MismatchedTypes, + LengthsDiffer, + KeysDiffer, + InconsistentGlyphOrder, + InconsistentExtensions, + UnsupportedFormat, + UnsupportedFormat, VarLibMergeError, ) @@ -79,7 +79,7 @@ class Merger(object): item.ensureDecompiled() keys = sorted(vars(out).keys()) if not all(keys == sorted(vars(v).keys()) for v in lst): - raise VarLibMergeFailureKeysDiffer(self, ({ + raise KeysDiffer(self, ({ "expected": keys, "got": [sorted(vars(v).keys()) for v in lst]},)) mergers = self.mergersFor(out) @@ -97,7 +97,7 @@ class Merger(object): def mergeLists(self, out, lst): if not allEqualTo(out, lst, len): - raise VarLibMergeFailureLengthsDiffer(self, ({ + raise LengthsDiffer(self, ({ "expected": len(out), "got": [len(x) for x in lst]},)) for i,(value,values) in enumerate(zip(out, zip(*lst))): @@ -109,7 +109,7 @@ class Merger(object): def mergeThings(self, out, lst): if not allEqualTo(out, lst, type): - raise VarLibMergeFailureMismatchedTypes(self, ({ + raise MismatchedTypes(self, ({ "expected": type(out), "got": [type(x) for x in lst]},)) mergerFunc = self.mergersFor(out).get(None, None) @@ -121,7 +121,7 @@ class Merger(object): self.mergeLists(out, lst) else: if not allEqualTo(out, lst): - raise VarLibMergeFailureShouldBeConstant(self, ({ + raise ShouldBeConstant(self, ({ "expected": out, "got": lst},)) @@ -146,7 +146,7 @@ class AligningMerger(Merger): def merge(merger, self, lst): if self is None: if not allNone(lst): - raise VarLibMergeFailureNotANone(self, ({ + raise NotANone(self, ({ "expected": None, "got": lst },)) @@ -162,7 +162,7 @@ def merge(merger, self, lst): for k in allKeys: allValues = nonNone(l.get(k) for l in lst) if not allEqual(allValues): - raise VarLibMergeFailureShouldBeConstant(self, ({ + raise ShouldBeConstant(self, ({ "expected": allValues[0], "got": lst, }, "."+k)) @@ -202,7 +202,7 @@ def _merge_GlyphOrders(font, lst, values_lst=None, default=None): order = sorted(combined, key=sortKey) # Make sure all input glyphsets were in proper order if not all(sorted(vs, key=sortKey) == vs for vs in lst): - raise VarLibMergeFailureInconsistentGlyphOrder(self, ({ + raise InconsistentGlyphOrder(self, ({ },)) del combined @@ -230,7 +230,7 @@ def _Lookup_SinglePos_get_effective_value(subtables, glyph): elif self.Format == 2: return self.Value[self.Coverage.glyphs.index(glyph)] else: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "single positioning lookup" },)) return None @@ -254,7 +254,7 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0) return self.Class1Record[klass1].Class2Record[klass2] else: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "pair positioning lookup" },)) return None @@ -263,7 +263,7 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) def merge(merger, self, lst): self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0) if not (len(lst) == 1 or (valueFormat & ~0xF == 0)): - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "single positioning lookup" },)) @@ -552,7 +552,7 @@ def merge(merger, self, lst): elif self.Format == 2: _PairPosFormat2_merge(self, lst, merger) else: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "pair positioning lookup" },)) @@ -669,28 +669,28 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): @AligningMerger.merger(ot.MarkBasePos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise VarLibMergeFailureInconsistentFormats(self, ({ + raise InconsistentFormats(self, ({ "subtable": "mark-to-base positioning lookup", "expected": self.Format, "got": [l.Format for l in lst]},)) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger) else: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "mark-to-base positioning lookup", })) @AligningMerger.merger(ot.MarkMarkPos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise VarLibMergeFailureInconsistentFormats(self, ({ + raise InconsistentFormats(self, ({ "subtable": "mark-to-mark positioning lookup", "expected": self.Format, "got": [l.Format for l in lst]},)) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2') else: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "mark-to-mark positioning lookup", })) @@ -822,12 +822,12 @@ def merge(merger, self, lst): continue if sts[0].__class__.__name__.startswith('Extension'): if not allEqual([st.__class__ for st in sts]): - raise VarLibMergeFailureInconsistentExtensions(self, ({ + raise InconsistentExtensions(self, ({ "expected": "Extension", "got": [st.__class__.__name__ for st in sts] },)) if not allEqual([st.ExtensionLookupType for st in sts]): - raise VarLibMergeFailureInconsistentExtensions(self, ({ + raise InconsistentExtensions(self, ({ },)) l.LookupType = sts[0].ExtensionLookupType new_sts = [st.ExtSubTable for st in sts] @@ -1057,7 +1057,7 @@ class VariationMerger(AligningMerger): if None in lst: if allNone(lst): if out is not None: - raise VarLibMergeFailureFoundANone(self, ({ + raise FoundANone(self, ({ "got": lst },)) return @@ -1080,7 +1080,7 @@ def buildVarDevTable(store_builder, master_values): @VariationMerger.merger(ot.BaseCoord) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "a baseline coordinate" },)) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) @@ -1091,7 +1091,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.CaretValue) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "a caret" },)) self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) @@ -1102,7 +1102,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.Anchor) def merge(merger, self, lst): if self.Format != 1: - raise VarLibMergeFailureUnsupportedFormat(self, ({ + raise UnsupportedFormat(self, ({ "subtable": "an anchor" },)) self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst]) From 956087eb81237465f1a590887259fea13d7860bd Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 18 Mar 2021 21:23:34 +0000 Subject: [PATCH 15/20] =?UTF-8?q?super=20doesn=E2=80=99t=20magically=20cal?= =?UTF-8?q?l=20its=20own=20methods?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Lib/fontTools/varLib/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index daaaef602..9d150a0e0 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -78,7 +78,7 @@ class ShouldBeConstant(VarLibMergeError): @property def details(self): if self.stack[0] != ".FeatureCount": - return super() + return super().details offender_index, offender = self.offender bad_ttf = self.merger.ttfs[offender_index] good_ttf = self.merger.ttfs[offender_index - 1] From 8ba31b0a43a939ecc797072b98cbbde293576352 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 18 Mar 2021 21:23:40 +0000 Subject: [PATCH 16/20] Improve error message --- Lib/fontTools/varLib/merger.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index ffe4cf8b1..72868c6c2 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -110,8 +110,8 @@ class Merger(object): def mergeThings(self, out, lst): if not allEqualTo(out, lst, type): raise MismatchedTypes(self, ({ - "expected": type(out), - "got": [type(x) for x in lst]},)) + "expected": type(out).__name__, + "got": [type(x).__name__ for x in lst]},)) mergerFunc = self.mergersFor(out).get(None, None) if mergerFunc is not None: mergerFunc(self, out, lst) From 23cb8b989bf6cad4654da42aca65ff9349bf08ca Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 18 Mar 2021 21:24:13 +0000 Subject: [PATCH 17/20] More tests --- .../data/IncompatibleArrays.designspace | 22 + .../data/IncompatibleLookupTypes.designspace | 22 + .../IncompatibleArrays-Bold.ttx | 612 +++++++++++++++++ .../IncompatibleArrays-Regular.ttx | 626 ++++++++++++++++++ .../IncompatibleLookupTypes-Bold.ttx | 622 +++++++++++++++++ .../IncompatibleLookupTypes-Regular.ttx | 626 ++++++++++++++++++ Tests/varLib/varLib_test.py | 38 +- 7 files changed, 2566 insertions(+), 2 deletions(-) create mode 100644 Tests/varLib/data/IncompatibleArrays.designspace create mode 100644 Tests/varLib/data/IncompatibleLookupTypes.designspace create mode 100644 Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Bold.ttx create mode 100644 Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Regular.ttx create mode 100644 Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Bold.ttx create mode 100644 Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Regular.ttx diff --git a/Tests/varLib/data/IncompatibleArrays.designspace b/Tests/varLib/data/IncompatibleArrays.designspace new file mode 100644 index 000000000..399810ea0 --- /dev/null +++ b/Tests/varLib/data/IncompatibleArrays.designspace @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/IncompatibleLookupTypes.designspace b/Tests/varLib/data/IncompatibleLookupTypes.designspace new file mode 100644 index 000000000..c7d357549 --- /dev/null +++ b/Tests/varLib/data/IncompatibleLookupTypes.designspace @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Bold.ttx b/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Bold.ttx new file mode 100644 index 000000000..1869dd426 --- /dev/null +++ b/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Bold.ttx @@ -0,0 +1,612 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Bold + + + 1.000;NONE;SimpleTwoAxis-Bold + + + Simple Two Axis Bold + + + Version 1.000 + + + SimpleTwoAxis-Bold + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Regular.ttx b/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Regular.ttx new file mode 100644 index 000000000..7186a3e97 --- /dev/null +++ b/Tests/varLib/data/master_incompatible_arrays/IncompatibleArrays-Regular.ttx @@ -0,0 +1,626 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Regular + + + 1.000;NONE;SimpleTwoAxis-Regular + + + Simple Two Axis Regular + + + Version 1.000 + + + SimpleTwoAxis-Regular + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Bold.ttx b/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Bold.ttx new file mode 100644 index 000000000..cf25178da --- /dev/null +++ b/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Bold.ttx @@ -0,0 +1,622 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Bold + + + 1.000;NONE;SimpleTwoAxis-Bold + + + Simple Two Axis Bold + + + Version 1.000 + + + SimpleTwoAxis-Bold + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Regular.ttx b/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Regular.ttx new file mode 100644 index 000000000..b1aac860b --- /dev/null +++ b/Tests/varLib/data/master_incompatible_lookup_types/IncompatibleLookupTypes-Regular.ttx @@ -0,0 +1,626 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Simple Two Axis + + + Regular + + + 1.000;NONE;SimpleTwoAxis-Regular + + + Simple Two Axis Regular + + + Version 1.000 + + + SimpleTwoAxis-Regular + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 2a4cff767..0be1c99de 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -1,7 +1,8 @@ from fontTools.misc.py23 import * from fontTools.ttLib import TTFont, newTable from fontTools.varLib import build, load_designspace -from fontTools.varLib.errors import VarLibValidationError, VarLibMergeError +from fontTools.varLib.errors import VarLibValidationError +import fontTools.varLib.errors as varLibErrors from fontTools.varLib.mutator import instantiateVariableFont from fontTools.varLib import main as varLib_main, load_masters from fontTools.varLib import set_default_weight_width_slant @@ -815,7 +816,7 @@ class BuildTest(unittest.TestCase): def test_varlib_build_incompatible_features(self): with pytest.raises( - VarLibMergeError, + varLibErrors.ShouldBeConstant, match = """ Couldn't merge the fonts, because some values were different, but should have @@ -837,6 +838,39 @@ Got: kern. save_before_dump=True, ) + def test_varlib_build_incompatible_lookup_types(self): + with pytest.raises( + varLibErrors.MismatchedTypes, + match = r"MarkBasePos, instead saw PairPos" + ): + self._run_varlib_build_test( + designspace_name="IncompatibleLookupTypes", + font_name="IncompatibleLookupTypes", + tables=["GPOS"], + expected_ttx_name="IncompatibleLookupTypes", + save_before_dump=True, + ) + + def test_varlib_build_incompatible_arrays(self): + with pytest.raises( + varLibErrors.ShouldBeConstant, + match = """ + +Couldn't merge the fonts, because some values were different, but should have +been the same. This happened while performing the following operation: +GPOS.table.ScriptList.ScriptCount + +The problem is likely to be in Simple Two Axis Bold: +Expected to see .ScriptCount==1, instead saw 0""" + ): + self._run_varlib_build_test( + designspace_name="IncompatibleArrays", + font_name="IncompatibleArrays", + tables=["GPOS"], + expected_ttx_name="IncompatibleArrays", + save_before_dump=True, + ) + def test_load_masters_layerName_without_required_font(): ds = DesignSpaceDocument() s = SourceDescriptor() From bfe4bad37b93904619d66866b696a07986cec927 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Fri, 19 Mar 2021 10:38:15 +0000 Subject: [PATCH 18/20] Use kwargs and explicit stack --- Lib/fontTools/varLib/errors.py | 19 +++--- Lib/fontTools/varLib/merger.py | 111 ++++++++++++--------------------- 2 files changed, 49 insertions(+), 81 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index 9d150a0e0..e14138290 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -12,17 +12,16 @@ class VarLibValidationError(VarLibError): class VarLibMergeError(VarLibError): """Raised when input data cannot be merged into a variable font.""" - def __init__(self, merger, args): + def __init__(self, merger, **kwargs): self.merger = merger - self.args = args - - @property - def cause(self): - return self.args[0] - - @property - def stack(self): - return self.args[1:] + if not kwargs: + kwargs = {} + if "stack" in kwargs: + self.stack = kwargs["stack"] + del kwargs["stack"] + else: + self.stack = [] + self.cause = kwargs @property def reason(self): diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index 72868c6c2..b8b7b27d6 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -79,9 +79,9 @@ class Merger(object): item.ensureDecompiled() keys = sorted(vars(out).keys()) if not all(keys == sorted(vars(v).keys()) for v in lst): - raise KeysDiffer(self, ({ - "expected": keys, - "got": [sorted(vars(v).keys()) for v in lst]},)) + raise KeysDiffer(self, expected=keys, + got=[sorted(vars(v).keys()) for v in lst] + ) mergers = self.mergersFor(out) defaultMerger = mergers.get('*', self.__class__.mergeThings) try: @@ -92,26 +92,25 @@ class Merger(object): mergerFunc = mergers.get(key, defaultMerger) mergerFunc(self, value, values) except VarLibMergeError as e: - e.args = e.args + ('.'+key,) + e.stack.append('.'+key) raise def mergeLists(self, out, lst): if not allEqualTo(out, lst, len): - raise LengthsDiffer(self, ({ - "expected": len(out), - "got": [len(x) for x in lst]},)) + raise LengthsDiffer(self, expected=len(out), got=[len(x) for x in lst]) for i,(value,values) in enumerate(zip(out, zip(*lst))): try: self.mergeThings(value, values) except VarLibMergeError as e: - e.args = e.args + ('[%d]' % i,) + e.stack.append('[%d]' % i) raise def mergeThings(self, out, lst): if not allEqualTo(out, lst, type): - raise MismatchedTypes(self, ({ - "expected": type(out).__name__, - "got": [type(x).__name__ for x in lst]},)) + raise MismatchedTypes(self, + expected=type(out).__name__, + got=[type(x).__name__ for x in lst] + ) mergerFunc = self.mergersFor(out).get(None, None) if mergerFunc is not None: mergerFunc(self, out, lst) @@ -121,9 +120,7 @@ class Merger(object): self.mergeLists(out, lst) else: if not allEqualTo(out, lst): - raise ShouldBeConstant(self, ({ - "expected": out, - "got": lst},)) + raise ShouldBeConstant(self, expected=out, got=lst) def mergeTables(self, font, master_ttfs, tableTags): for tag in tableTags: @@ -133,7 +130,7 @@ class Merger(object): self.mergeThings(font[tag], [m[tag] if tag in m else None for m in master_ttfs]) except VarLibMergeError as e: - e.args = e.args + (tag,) + e.stack.append(tag) raise # @@ -146,10 +143,7 @@ class AligningMerger(Merger): def merge(merger, self, lst): if self is None: if not allNone(lst): - raise NotANone(self, ({ - "expected": None, - "got": lst - },)) + raise NotANone(self, expected=None, got=lst) return lst = [l.classDefs for l in lst] @@ -162,10 +156,7 @@ def merge(merger, self, lst): for k in allKeys: allValues = nonNone(l.get(k) for l in lst) if not allEqual(allValues): - raise ShouldBeConstant(self, ({ - "expected": allValues[0], - "got": lst, - }, "."+k)) + raise ShouldBeConstant(self, expected=allValues[0], got=lst, stack="."+k) if not allValues: self[k] = None else: @@ -202,8 +193,7 @@ def _merge_GlyphOrders(font, lst, values_lst=None, default=None): order = sorted(combined, key=sortKey) # Make sure all input glyphsets were in proper order if not all(sorted(vs, key=sortKey) == vs for vs in lst): - raise InconsistentGlyphOrder(self, ({ - },)) + raise InconsistentGlyphOrder(self) del combined paddedValues = None @@ -230,9 +220,7 @@ def _Lookup_SinglePos_get_effective_value(subtables, glyph): elif self.Format == 2: return self.Value[self.Coverage.glyphs.index(glyph)] else: - raise UnsupportedFormat(self, ({ - "subtable": "single positioning lookup" - },)) + raise UnsupportedFormat(self, subtable="single positioning lookup") return None def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph): @@ -254,19 +242,14 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0) return self.Class1Record[klass1].Class2Record[klass2] else: - raise UnsupportedFormat(self, ({ - "subtable": "pair positioning lookup" - },)) + raise UnsupportedFormat(self, subtable="pair positioning lookup") return None @AligningMerger.merger(ot.SinglePos) def merge(merger, self, lst): self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0) if not (len(lst) == 1 or (valueFormat & ~0xF == 0)): - raise UnsupportedFormat(self, ({ - "subtable": "single positioning lookup" - },)) - + raise UnsupportedFormat(self, subtable="single positioning lookup") # If all have same coverage table and all are format 1, coverageGlyphs = self.Coverage.glyphs @@ -552,9 +535,7 @@ def merge(merger, self, lst): elif self.Format == 2: _PairPosFormat2_merge(self, lst, merger) else: - raise UnsupportedFormat(self, ({ - "subtable": "pair positioning lookup" - },)) + raise UnsupportedFormat(self, subtable="pair positioning lookup") del merger.valueFormat1, merger.valueFormat2 @@ -669,31 +650,28 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): @AligningMerger.merger(ot.MarkBasePos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise InconsistentFormats(self, ({ - "subtable": "mark-to-base positioning lookup", - "expected": self.Format, - "got": [l.Format for l in lst]},)) + raise InconsistentFormats(self, + subtable="mark-to-base positioning lookup", + expected=self.Format, + got=[l.Format for l in lst] + ) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger) else: - raise UnsupportedFormat(self, ({ - "subtable": "mark-to-base positioning lookup", - })) + raise UnsupportedFormat(self, subtable="mark-to-base positioning lookup") @AligningMerger.merger(ot.MarkMarkPos) def merge(merger, self, lst): if not allEqualTo(self.Format, (l.Format for l in lst)): - raise InconsistentFormats(self, ({ - "subtable": "mark-to-mark positioning lookup", - "expected": self.Format, - "got": [l.Format for l in lst]},)) + raise InconsistentFormats(self, + subtable="mark-to-mark positioning lookup", + expected=self.Format, + got=[l.Format for l in lst] + ) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2') else: - raise UnsupportedFormat(self, ({ - "subtable": "mark-to-mark positioning lookup", - })) - + raise UnsupportedFormat(self, subtable="mark-to-mark positioning lookup") def _PairSet_flatten(lst, font): self = ot.PairSet() @@ -822,13 +800,12 @@ def merge(merger, self, lst): continue if sts[0].__class__.__name__.startswith('Extension'): if not allEqual([st.__class__ for st in sts]): - raise InconsistentExtensions(self, ({ - "expected": "Extension", - "got": [st.__class__.__name__ for st in sts] - },)) + raise InconsistentExtensions(self, + expected="Extension", + got=[st.__class__.__name__ for st in sts] + ) if not allEqual([st.ExtensionLookupType for st in sts]): - raise InconsistentExtensions(self, ({ - },)) + raise InconsistentExtensions(self) l.LookupType = sts[0].ExtensionLookupType new_sts = [st.ExtSubTable for st in sts] del sts[:] @@ -1057,9 +1034,7 @@ class VariationMerger(AligningMerger): if None in lst: if allNone(lst): if out is not None: - raise FoundANone(self, ({ - "got": lst - },)) + raise FoundANone(self, got=lst) return masterModel = self.model model, lst = masterModel.getSubModel(lst) @@ -1080,9 +1055,7 @@ def buildVarDevTable(store_builder, master_values): @VariationMerger.merger(ot.BaseCoord) def merge(merger, self, lst): if self.Format != 1: - raise UnsupportedFormat(self, ({ - "subtable": "a baseline coordinate" - },)) + raise UnsupportedFormat(self, subtable="a baseline coordinate") self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) if DeviceTable: self.Format = 3 @@ -1091,9 +1064,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.CaretValue) def merge(merger, self, lst): if self.Format != 1: - raise UnsupportedFormat(self, ({ - "subtable": "a caret" - },)) + raise UnsupportedFormat(self, subtable="a caret") self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) if DeviceTable: self.Format = 3 @@ -1102,9 +1073,7 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.Anchor) def merge(merger, self, lst): if self.Format != 1: - raise UnsupportedFormat(self, ({ - "subtable": "an anchor" - },)) + raise UnsupportedFormat(self, subtable="an anchor") self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst]) self.YCoordinate, YDeviceTable = buildVarDevTable(merger.store_builder, [a.YCoordinate for a in lst]) if XDeviceTable or YDeviceTable: From 5d7c826b0b73ea2f2495e5e3f7462987ee3024c5 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Fri, 19 Mar 2021 10:40:02 +0000 Subject: [PATCH 19/20] Make CFF merge errors inherit differently --- Lib/fontTools/varLib/errors.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index e14138290..a2108ea36 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -156,8 +156,11 @@ class UnsupportedFormat(UnsupportedFormat): pass +class VarLibCFFMergeError(VarLibError): + pass -class VarLibCFFDictMergeError(VarLibMergeError): + +class VarLibCFFDictMergeError(VarLibCFFMergeError): """Raised when a CFF PrivateDict cannot be merged.""" def __init__(self, key, value, values): @@ -170,7 +173,7 @@ class VarLibCFFDictMergeError(VarLibMergeError): self.args = (error_msg,) -class VarLibCFFPointTypeMergeError(VarLibMergeError): +class VarLibCFFPointTypeMergeError(VarLibCFFMergeError): """Raised when a CFF glyph cannot be merged because of point type differences.""" def __init__(self, point_type, pt_index, m_index, default_type, glyph_name): @@ -182,7 +185,7 @@ class VarLibCFFPointTypeMergeError(VarLibMergeError): self.args = (error_msg,) -class VarLibCFFHintTypeMergeError(VarLibMergeError): +class VarLibCFFHintTypeMergeError(VarLibCFFMergeError): """Raised when a CFF glyph cannot be merged because of hint type differences.""" def __init__(self, hint_type, cmd_index, m_index, default_type, glyph_name): From baf6c5d1e10cc48f002d319be5b572e38bd573b8 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Fri, 19 Mar 2021 10:48:58 +0000 Subject: [PATCH 20/20] Remove pass if we have a docstring --- Lib/fontTools/varLib/errors.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py index a2108ea36..5840070f7 100644 --- a/Lib/fontTools/varLib/errors.py +++ b/Lib/fontTools/varLib/errors.py @@ -115,32 +115,22 @@ class FoundANone(VarLibMergeError): class MismatchedTypes(VarLibMergeError): """data had inconsistent types""" - pass - class LengthsDiffer(VarLibMergeError): """a list of objects had inconsistent lengths""" - pass - class KeysDiffer(VarLibMergeError): """a list of objects had different keys""" - pass - class InconsistentGlyphOrder(VarLibMergeError): """the glyph order was inconsistent between masters""" - pass - class InconsistentExtensions(VarLibMergeError): """the masters use extension lookups in inconsistent ways""" - pass - class UnsupportedFormat(VarLibMergeError): """an OpenType subtable (%s) had a format I didn't expect""" @@ -154,7 +144,6 @@ class UnsupportedFormat(VarLibMergeError): class UnsupportedFormat(UnsupportedFormat): """an OpenType subtable (%s) had inconsistent formats between masters""" - pass class VarLibCFFMergeError(VarLibError): pass