add support for chapter level usfm filtering and remarks#1021
Conversation
There was a problem hiding this comment.
Looks good!
I think we can assume that \c 1 always exists. I've never seen it omitted I don't think. And if it were omitted, I think it would cause other problems in our tools. Maybe you could ask the EITL team if they've seen this before if you'd like to double-check?
I'm fine with only updating translate_book, but that's more a question for Ben and Damien, I guess.
@Enkidu93 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on ddaspit).
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on mshannon-sil).
benjaminking
left a comment
There was a problem hiding this comment.
I'm fine with not having this behavior apply to the translate_usfm path, since it doesn't include a way to pass chapters, as you said.
@benjaminking reviewed all commit messages and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on mshannon-sil).
This PR addresses issue #836 and is dependent on changes in machine.py that are in this PR: sillsdev/machine.py#285.
This passes the chapters argument into the
update_usfmmethod from machine.py so that any usfm markers that are not the \id marker or in the requested chapters are omitted. It also updates remarks to be applied at a chapter level rather than at a book level.A couple questions/notes:
\c 1marker, or is it possible projects may omit it? Because the way the remarks algorithm now works for a book like Jude, it creates a remark for chapter 1, but the remark won't be applied if there is no\c 1marker.translate_usfmmethod, since the way silnlp is set up it doesn't get used when books/chapters are specified. Also, updating it to support chapters would include having to import more from machine.py, particularlyfilter_tokens_by_chapterthat we didn't want to import if we didn't have to. If I should still add support for chapter level drafting to this manual version regardless, please let me know.This change is