Skip to content

Conversation

benjaminking
Copy link
Collaborator

@benjaminking benjaminking commented Sep 29, 2025

This PR addresses Issue #810, allowing multiple translations to work with confidence score calculations. Due to the extra parameters needed from the translator for calculating confidence scores, this involved a fair bit of refactoring.


This change is Reviewable

Copy link
Collaborator

@mshannon-sil mshannon-sil left a comment

Choose a reason for hiding this comment

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

@mshannon-sil reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @benjaminking)


silnlp/common/translator.py line 277 at r1 (raw file):

            else:
                trg_draft_file_path = trg_file_path
            write_corpus(trg_draft_file_path, translated_draft.get_all_translations())

I might just be missing it, but I did a search of the repo in both the master and this branch, and I don't see a method corresponding to get_all_translations()

Copy link
Collaborator Author

@benjaminking benjaminking left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @mshannon-sil)


silnlp/common/translator.py line 277 at r1 (raw file):

Previously, mshannon-sil wrote…

I might just be missing it, but I did a search of the repo in both the master and this branch, and I don't see a method corresponding to get_all_translations()

Good catch! I had meant to rename a method and forgot to do that. I've fixed it and tested that it works with text file translation.

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.

2 participants