Skip to content

Conversation

benjaminking
Copy link
Collaborator

@benjaminking benjaminking commented Sep 23, 2025

This PR updates the quotation denormalizer to only make use the target corpus quote convention.

(Eli and I were discussing this the last few days and concluded that it's more appropriate to use the normalized version of the target corpus quote convention in place of the source corpus quote convention)

This also includes some reorganization, since moving the punctuation_analysis package was resulting in circular dependencies.


This change is Reviewable

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 23 of 23 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Enkidu93)


tests/punctuation_analysis/test_paratext_project_quote_convention_detector.py line 6 at r1 (raw file):

from machine.corpora import ParatextProjectSettings, UsfmStylesheet
from machine.punctuation_analysis.paratext_project_quote_convention_detector import (

These should be simplified to importing from machine.punctuation_analysis instead of the modules.


tests/testutils/memory_paratext_project_quote_convention_detector.py line 5 at r1 (raw file):

from machine.corpora import ParatextProjectSettings
from machine.punctuation_analysis.paratext_project_quote_convention_detector import (

This should be simplified to importing from machine.punctuation_analysis instead of the module.

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: 4 of 24 files reviewed, 2 unresolved discussions (waiting on @ddaspit and @Enkidu93)


tests/testutils/memory_paratext_project_quote_convention_detector.py line 5 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This should be simplified to importing from machine.punctuation_analysis instead of the module.

Done.


tests/punctuation_analysis/test_paratext_project_quote_convention_detector.py line 6 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

These should be simplified to importing from machine.punctuation_analysis instead of the modules.

Done.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.91%. Comparing base (852ea41) to head (7758d2f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
- Coverage   90.91%   90.91%   -0.01%     
==========================================
  Files         337      337              
  Lines       21524    21519       -5     
==========================================
- Hits        19569    19564       -5     
  Misses       1955     1955              

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

:lgtm:

@ddaspit reviewed 20 of 20 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

Copy link
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.

:lgtm:

@Enkidu93 reviewed 4 of 23 files at r1, 20 of 20 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @benjaminking)

@benjaminking benjaminking merged commit d93dacd into main Sep 26, 2025
14 checks passed
@benjaminking benjaminking deleted the remove_source_quote_convention branch September 26, 2025 20:44
Enkidu93 added a commit to sillsdev/machine that referenced this pull request Sep 29, 2025
Enkidu93 added a commit to sillsdev/machine that referenced this pull request Sep 30, 2025
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.

4 participants