Skip to content

Conversation

@Enkidu93
Copy link
Collaborator

@Enkidu93 Enkidu93 commented Feb 6, 2026

Hopefully there aren't too many things we've missed along the way. There are fewer things that need to be ported to Machine from machine.py luckily although I was surprised to discover that #141 has never been ported 🤪.


This change is Reviewable

@Enkidu93 Enkidu93 requested a review from ddaspit February 6, 2026 22:42
@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2026

Codecov Report

❌ Patch coverage is 96.90265% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.75%. Comparing base (f1dc4f1) to head (a8544a8).

Files with missing lines Patch % Lines
...atext_project_versification_error_detector_base.py 42.85% 4 Missing ⚠️
.../corpora/scripture_ref_usfm_parser_handler_base.py 93.33% 2 Missing ⚠️
machine/corpora/paratext_project_settings.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   90.68%   90.75%   +0.07%     
==========================================
  Files         355      355              
  Lines       22508    22677     +169     
==========================================
+ Hits        20411    20581     +170     
+ Misses       2097     2096       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

@ddaspit partially reviewed 19 files and all commit messages, and made 2 comments.
Reviewable status: 19 of 20 files reviewed, 2 unresolved discussions (waiting on @Enkidu93).


machine/corpora/scripture_ref_usfm_parser_handler_base.py line 28 at r1 (raw file):

def is_private_use_marker(marker: str):

This function is private to the module, i.e. you should prefix with an underscore.


machine/corpora/update_usfm_parser_handler.py line 41 at r1 (raw file):

def sanitize_verse_data(verse_data: str) -> str:

This function is private to the module.

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

@ddaspit reviewed 1 file and made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Enkidu93).


machine/scripture/verse_ref.py line 152 at r1 (raw file):

    @verse_num.setter
    def verse_num(self, value: int) -> None:
        if value < 0:

Why do we need to remove this check? It deviates from the C# code.

Copy link
Collaborator Author

@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.

@Enkidu93 made 3 comments.
Reviewable status: 19 of 21 files reviewed, 3 unresolved discussions (waiting on @ddaspit).


machine/corpora/update_usfm_parser_handler.py line 41 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This function is private to the module.

Done.


machine/scripture/verse_ref.py line 152 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

Why do we need to remove this check? It deviates from the C# code.

We do not need to. I misread and thought this check was on the verse setter which (for better or worse) is settable to something like -1. I reverted it.


machine/corpora/scripture_ref_usfm_parser_handler_base.py line 28 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This function is private to the module, i.e. you should prefix with an underscore.

Done.

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ddaspit partially reviewed 5 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93).

@Enkidu93
Copy link
Collaborator Author

Fixes #265

@Enkidu93 Enkidu93 merged commit 1c6bde6 into main Feb 10, 2026
14 checks passed
@Enkidu93 Enkidu93 deleted the port_updates branch February 10, 2026 14:19
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.

3 participants