Expose SegmentBoundaryAdjuster for external use - #345
Conversation
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on Enkidu93).
Enkidu93
left a comment
There was a problem hiding this comment.
@Enkidu93 reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on benjaminking).
|
@benjaminking, did you already try pushing an empty commit to see if that would trigger the CI? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
=======================================
Coverage 91.94% 91.94%
=======================================
Files 387 387
Lines 24380 24381 +1
=======================================
+ Hits 22415 22416 +1
Misses 1965 1965 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Enkidu93
left a comment
There was a problem hiding this comment.
Looks good! I need to put a PR into silnlp to work with the new machine.py API - just so you know: If we release and you try to use this version, python will yell at you a little til that's in.
@Enkidu93 reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on benjaminking).
benjaminking
left a comment
There was a problem hiding this comment.
Sorry, can you clarify: what do I need to get in?
@benjaminking made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on benjaminking).
Enkidu93
left a comment
There was a problem hiding this comment.
Sorry - to be more specific: The API for the USFM versification warnings has changed, so if we release a new version of machine.py in order to expose the segment boundary adjuster and update silnlp to use the new version, it will break silnlp. I am planning on pushing an update to silnlp soon - it just keeps slipping out of mind.
@Enkidu93 made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on benjaminking).
benjaminking
left a comment
There was a problem hiding this comment.
Ah, okay. I understand now. We can try to coordinate those two changes.
@benjaminking made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on benjaminking).
This PR exposes
SegmentBoundaryAdjusterso that it can be used by external callers, specifically the verse segmenter in SILNLP.This change is