Add chapter-level data counts to execution data#990
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #990 +/- ##
==========================================
- Coverage 65.47% 65.34% -0.13%
==========================================
Files 387 388 +1
Lines 21747 21932 +185
Branches 2770 2782 +12
==========================================
+ Hits 14238 14332 +94
- Misses 6472 6560 +88
- Partials 1037 1040 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I guess I should add this to Echo as well. I'll add that. |
I'm realizing that I'll need a Machine dependency in Echo in order to do that. Is that OK or would you prefer to just wait until we complete #987 and let it be resolved there? |
2363b49 to
e52b0c5
Compare
|
Our conclusion was that we will only count scripture segments in this new counts data structure. |
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 20 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Enkidu93).
src/Machine/src/Serval.Machine.Shared/Models/BuildExecutionData.cs line 9 at r1 (raw file):
public bool? IsTrainFilteredByChapter { get; init; } public bool? IsInferenceFilteredByChapter { get; init; } public Dictionary<string, Dictionary<string, int>>? TrainVerseCount { get; init; }
Can we use IReadOnlyDictionary? Does it cause issues?
Fixes #907
@ddaspit, my only outstanding question is how we'd like to count only pretranslated non-verse text. At the moment, they're all lumped together. We could either not count them at all or count them separately or just lump them in (current behavior). What do you think makes the most sense?
This change is