Port machine changes

[Enkidu93]

Port

• Match SILNLP's term rendering and gloss cleaning exactly machine#343
• Add update block error handling machine#342
• Get update rows by reference machine#341

Sorry this took so long - I hit a bug that I just couldn't figure out since I was certain I had ported the relevant code correctly. Turns out that machine.py and Machine code had become slightly out of sync. I went ahead and made the change here to make it consistent with Machine, but I'm suspicious that Machine was actually the incorrect one/missing a commit. I think that maybe #170 never got entirely properly incorporated into Machine because it went in around the same time as the update block code. Could you take a peek, @ddaspit, and let me know what's right and what's wrong? Also happy to just push this through since the tests are passing and follow up on this in a separate issue.

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 92.71523% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.91%. Comparing base (7021586) to head (2c3ec99).

Files with missing lines	Patch %	Lines
machine/corpora/update_usfm_parser_handler.py	95.06%	4 Missing ⚠️
machine/corpora/usfm_update_block_handler.py	57.14%	3 Missing ⚠️
...corpora/place_markers_usfm_update_block_handler.py	66.66%	2 Missing ⚠️
...hine/corpora/paratext_project_terms_parser_base.py	94.11%	1 Missing ⚠️
machine/scripture/verse_ref.py	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
- Coverage   90.92%   90.91%   -0.01%     
==========================================
  Files         337      337              
  Lines       21542    21642     +100     
==========================================
+ Hits        19586    19675      +89     
- Misses       1956     1967      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ddaspit]

@ddaspit reviewed 8 of 10 files at r1, all commit messages.
Reviewable status: 8 of 10 files reviewed, 1 unresolved discussion

---

machine/corpora/usfm_update_block_handler.py line 11 at r1 (raw file):

class UsfmUpdateBlockHandlerException(Exception):

You should name this UsfmUpdateBlockHandlerError so that it is consistent with Python naming conventions.

[Enkidu93]

Reviewable status: 4 of 10 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

machine/corpora/usfm_update_block_handler.py line 11 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You should name this UsfmUpdateBlockHandlerError so that it is consistent with Python naming conventions.

I wasn't sure about this. Done.

[ddaspit]

@ddaspit reviewed 2 of 10 files at r1.
Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @Enkidu93)

---

machine/corpora/scripture_ref.py line 125 at r1 (raw file):

        # Using to_relaxed() is necessary to maintain equality across relaxed refs,
        # __eq__ properly handles relaxed ref comparison
        return hash((self.verse_ref, tuple(self.to_relaxed().path)))

This is different than the implementation in Machine. On the other hand, Machine is inconsistent with itself, which is probably not a good thing. ScriptureRefComparer.GetHashCode() and ScriptureRef.GetHashCode() give different results, which would be unexpected for a caller. Why do we need to call to_relaxed here?

[ddaspit]

@ddaspit reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

[Enkidu93]

@ddaspit reviewed 2 of 10 files at r1.
Reviewable status: 6 of 10 files reviewed, 2 unresolved discussions (waiting on @Enkidu93)

machine/corpora/scripture_ref.py line 125 at r1 (raw file):

        # Using to_relaxed() is necessary to maintain equality across relaxed refs,
        # __eq__ properly handles relaxed ref comparison
        return hash((self.verse_ref, tuple(self.to_relaxed().path)))

This is different than the implementation in Machine. On the other hand, Machine is inconsistent with itself, which is probably not a good thing. ScriptureRefComparer.GetHashCode() and ScriptureRef.GetHashCode() give different results, which would be unexpected for a caller. Why do we need to call to_relaxed here?

That's a great question. Maybe the simplest thing to do is to just remove the scripture ref comparer since it's not being used with the current row-lookup solution. The reason that I used to_relaxed() is because of the intransitivity of ScriptureRef equality when you have, say, one relaxed ref and two non-relaxed refs with the same markers. If we want relaxed and non-relaxed refs to fall into the same bucket properly, we would need to fall back on __eq__ which handles this correctly. But as I think about this more, I'm not even sure that that would work - it would depend on how Python implements dict. What do you think?

[ddaspit]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

---

machine/corpora/scripture_ref.py line 125 at r1 (raw file):

Previously, Enkidu93 (Eli C. Lowry) wrote…

That's a great question. Maybe the simplest thing to do is to just remove the scripture ref comparer since it's not being used with the current row-lookup solution. The reason that I used to_relaxed() is because of the intransitivity of ScriptureRef equality when you have, say, one relaxed ref and two non-relaxed refs with the same markers. If we want relaxed and non-relaxed refs to fall into the same bucket properly, we would need to fall back on __eq__ which handles this correctly. But as I think about this more, I'm not even sure that that would work - it would depend on how Python implements dict. What do you think?

If we are not using it, let's go ahead and remove it.

[Enkidu93]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

machine/corpora/scripture_ref.py line 125 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

If we are not using it, let's go ahead and remove it.

Done.

[Enkidu93]

Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

machine/corpora/scripture_ref.py line 125 at r1 (raw file):

Previously, Enkidu93 (Eli C. Lowry) wrote…

Done.

Removing it in Machine here as well

[ddaspit]

@ddaspit reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)