Closed jillianchang closed 3 years ago
That looks like you did things right except the bit about line.meter
doesn't make any sense: we haven't defined a meter field yet.
The assertion failure is saying that the pronunciation string is empty, which happens when one of the compositions fails.
To see what the error was, comment out the second-to-last line in
scansion_test.py (the one that reads logging.disable("CRITICAL")
) and
re-run.
On Sat, Aug 14, 2021 at 8:09 PM jillianchang @.***> wrote:
@.**** commented on this pull request.
In grammars/meter.grm https://github.com/CUNY-CL/LatinScansion/pull/39#discussion_r689007138:
@@ -66,7 +72,13 @@ test_scan_3 = AssertEqual[
"kwidwe doleːns reːgiːna deũː tot wolwere kaːsuːs" @ METER,
"DSDSDS"
];
+# Muta cum liquida is split.
test_scan_4 = AssertEqual[
"kekropidaj jussiː miserũː septeːna kwotanniːs" @ METER,
"DSDSDS"
];
Is this right for adding unit tests?
def test_aen_6_21(self): # Scans line 6.21. text = "Cecropidae jussī (miserum!) septēna quotannīs" line = self.scan_line(text, 21) self.assertEqual(line.line_number, 21) self.assertEqual(line.text, text) self.assertEqual( line.norm, "cecropidae jussī miserum septēna quotannīs" ) self.assertEqual( line.pron, "kekropidaj jussiː miserũː septeːna kwotanniːs" ) self.assertEqual( line.meter, "DSDSDS" )
I'm getting this for the pron part:
Traceback (most recent call last):
File "scansion_test.py", line 49, in test_aen_6_21
self.assertEqual(
AssertionError: '' != 'kekropidaj jussiː miserũː septeːna kwotanniːs'
- kekropidaj jussiː miserũː septeːna kwotanniːs
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CUNY-CL/LatinScansion/pull/39#discussion_r689007138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG4OOVVVNXI7NIHHZVRYDT44ATJANCNFSM5CCJIGTQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .
That looks like you did things right except the bit about
line.meter
doesn't make any sense: we haven't defined a meter field yet.
Oh I see, we only have line.norm and line.pron in scansion.py. What's the way to test the meter scan?
The assertion failure is saying that the pronunciation string is empty, which happens when one of the compositions fails. To see what the error was, comment out the second-to-last line in scansion_test.py (the one that reads
logging.disable("CRITICAL")
) and re-run. …
The error says WARNING:root:Defective line (line 21): 'cecropidae jussī miserum septēna quotannīs'
That looks like you did things right except the bit about
line.meter
doesn't make any sense: we haven't defined a meter field yet.Oh I see, we only have line.norm and line.pron in scansion.py. What's the way to test the meter scan?
Haven't added that feature yet.
The assertion failure is saying that the pronunciation string is empty, which happens when one of the compositions fails. To see what the error was, comment out the second-to-last line in scansion_test.py (the one that reads
logging.disable("CRITICAL")
) and re-run. …The error says
WARNING:root:Defective line (line 21): 'cecropidae jussī miserum septēna quotannīs'
Yeah, okay, the grammar is still not permitting the splitting of an MCL.
Let me suggest: make a separate toy grammar that is simpler than this, then test out its behavior.
If you’re still stuck when I get back to this that’s what I’d do.
K
On Sun, Aug 15, 2021 at 11:47 AM jillianchang @.***> wrote:
@.**** commented on this pull request.
In grammars/meter.grm https://github.com/CUNY-CL/LatinScansion/pull/39#discussion_r689108219:
@@ -24,8 +24,8 @@ onset = Optimize[
"x", "mn", and "phth" onsets are found word-initially in Greek names.
CDRewrite[(s_cluster | "ks" | "mn" | "pt") : "O", i.BOW, i.NUCLEUS, sigma_star] @
- CDRewrite[(muta_cum_liquida | labiovelar |
- i.CONSONANT) : "O", "", i.NUCLEUS, sigma_star]
- CDRewrite[(labiovelar | i.CONSONANT) - i.LIQUID : "O", "", i.NUCLEUS, sigma_star] @
I think you are right, I just thought that maybe having liquid and stop liquid sequence as the tau could permit it to either map the "tr" in patrem to an onset or the "r" to onset. I wasn't sure how to do it but I knew that the original code had to be changed somehow since it wasn't allowing the split in the first place.
— You are receiving this because you commented.
Reply to this email directly, view it on GitHub https://github.com/CUNY-CL/LatinScansion/pull/39#discussion_r689108219, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG4ONURD3K7KULIW2574LT47ORBANCNFSM5CCJIGTQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .
Okay. I just wrote my toy grammar and to my surprise, it didn't work initially: it would refuse to split the MCL between coda and onset like I wanted it to. I had this inkling that maybe this had something to do with rule directionality so I tried right-to-left application ('rtl') and lo and behold, it worked exactly like I want it to.
My example is here: https://gist.github.com/kylebgorman/258bc055f333f1ab2cea7d131e0a1df2
Oh I see, thank you for that. I tried playing around with directionality and tried "sim," but didn't think to try "rtl."
I'm slightly embarassed that I've been doing this for almost two decades now and I have no clue why 'rtl'
is the right answer here ;)
Is the latest good to be merged?
Yes, this is great, merging now.
I know that there's something wrong with this new edit, since the combined grammar isn't able to scan lines as it used to. Do you have suggestions for how to improve this current meter grammar to allow the "variable" behavior?
Additionally, I had to delete all the intermediary tests, because I think the 'opt' and weight in the new muta cum liquida rule messes that up (?)