Remove .Format from Coverage, ClassDef, SingleSubst, LigatureSubst, AlternateSubst #2238
Conversation
…ubst The format values for those are automatically handled in postRead/preWrite to choose optimal format. As such, don't write them in XML. Reduces noise. #2236 (comment)
This reverts commit fb0c60c.
|
in mti_test.py, if you move the order in which you dump to ttx and compile the table (by moving the dump from built table before the table.compile), then you get the expected ttx output (without Formats). I think it's because the preWrite methods called by compile re-add the Format attrs, so if you dump after a compile you still get those.. diff --git a/Tests/mtiLib/mti_test.py b/Tests/mtiLib/mti_test.py
index 1666dd45..6704751e 100644
--- a/Tests/mtiLib/mti_test.py
+++ b/Tests/mtiLib/mti_test.py
@@ -174,13 +174,6 @@ class MtiTest(unittest.TestCase):
self.assertEqual(tableTag, table.tableTag)
tableTag = table.tableTag
- # Make sure it compiles.
- blob = table.compile(font)
-
- # Make sure it decompiles.
- decompiled = table.__class__()
- decompiled.decompile(blob, font)
-
# XML from built object.
writer = XMLWriter(StringIO(), newlinestr='\n')
writer.begintag(tableTag); writer.newline()
@@ -188,6 +181,13 @@ class MtiTest(unittest.TestCase):
writer.endtag(tableTag); writer.newline()
xml_built = writer.file.getvalue()
+ # Make sure it compiles.
+ blob = table.compile(font)
+
+ # Make sure it decompiles.
+ decompiled = table.__class__()
+ decompiled.decompile(blob, font)
+
# XML from decompiled object.
writer = XMLWriter(StringIO(), newlinestr='\n')
writer.begintag(tableTag); writer.newline() |
|
there's still one mtiLib test failing after I apply the above patch though.. Should you also |
|
Ah, thanks. Let's keep the test code in same order so we test this route. I'll add a comment as well. Will fix preWrites and MultipleSubst. |
|
Okay! Ready to review / merge. All tests pass. :) |
So; we have feaLib/mtiLib/otlLib/fontBuilder building Python objects, then there's saving to binary and back, and to XML and back. Would be good if we can centralize testing of all that :))). Right now each test module does it's own. But yeah, glad mti's had different order so I caught more to fix. |
|
I like the centralization idea but don't know enough to meaningfully review this PR or centralize anything in ttLib :o |
| table = self.preWrite(font) | ||
| deleteFormat = deleteFormat and hasattr(self, 'Format') |
There was a problem hiding this comment.
What does this double check do? Why not just delete it when you hasattr it at the end?
There was a problem hiding this comment.
it means "delete Format attribute if it wasn't there before calling preWrite and it's been added by the preWrite"
There was a problem hiding this comment.
So why delete it at the end of the function?
There was a problem hiding this comment.
it's used when serializing the tables, we only want to delete after compile is completed
There was a problem hiding this comment.
It's just a hack. Ideally compilation shouldn't modify the objects. The preWrite methods are modifying by adding .Format. Eventually we will remove that, by rewriting how format-switching table machinery works. But for now, to make it work, I let preWrite add .Format, but will delete it at the end of compile when they are not needed anymore.
Another place we modify objects during compilation is when we split large lookups. But that's for another day...
From fb0c60c#issuecomment-805132445
Initially ~470 tests failed. I fixed all; expect for 25 mtiLib tests still fail. I don't understand why. Help appreciated.