Skip to content

add support for chapter level usfm filtering and remarks#1021

Open
mshannon-sil wants to merge 1 commit into
masterfrom
idg
Open

add support for chapter level usfm filtering and remarks#1021
mshannon-sil wants to merge 1 commit into
masterfrom
idg

Conversation

@mshannon-sil
Copy link
Copy Markdown
Collaborator

@mshannon-sil mshannon-sil commented Apr 29, 2026

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_usfm method 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:

  • For books with a single chapter like Jude, are we okay always assuming that there will be a \c 1 marker, 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 1 marker.
  • I didn't update the more manual version of updating usfm in the translate_usfm method, 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, particularly filter_tokens_by_chapter that 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 Reviewable

Copy link
Copy Markdown
Collaborator

@Enkidu93 Enkidu93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: :shipit: complete! all files reviewed, all discussions resolved (waiting on ddaspit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Partial book drafts are difficult to import if the project has existing text for that book

2 participants