fix: resolve Python 3 migration runtime errors and compatibility issues

[tianjianjiang]

User description

This commit fixes critical bugs in the Python 3 modernization that were causing GitHub Actions workflow failures in the "Test McBopomofo" step.

Fixes in score_validator.py:

• Add missing 'phrases' parameter to 20+ seg_pick() calls (lines 155, 166, 168, 175, 198, 214, 229, 240, 242, 249, 273, 290, 306, 321, 332, 334, 341)
• Add missing 'phrases' parameter to three_char_walk() calls (lines 216, 292, 308)
• Fix incorrect walker function call: four_char_walk -> five_char_walk (line 275)
• Optimize control flow by converting multiple if statements to elif chain in check_bpmf_output() function to prevent redundant execution

Fixes in phrase_deriver.py:

• Remove zip(strict=True) for Python 3.9+ compatibility (strict parameter requires Python 3.10+)
• Length validation already performed before zip, so strict mode redundant

All fixes verified with:

• Syntax validation (py_compile)
• Full data build (make all)
• Data integrity checks (make check)
• Score validator execution

These changes resolve the TypeErrors that were preventing the workflow from completing successfully.

---

PR Type

Bug fix, Enhancement

---

Description

• Pass phrases to seg_pick and walkers

• Fix incorrect walker call in six_char path

• Optimize validator with single length branch

• Remove zip strict for Python 3.9+

---

Diagram Walkthrough

flowchart LR
  PhraseDeriver["phrase_deriver.py: remove zip strict=True"] -- "compatibility" --> Python39["Python 3.9+"]
  ScoreValidator["score_validator.py: seg_pick calls pass phrases"] -- "correct params" --> Walkers["two/three/four/five/six_char_walk"]
  ScoreValidator -- "fix call" --> SixChar["six_char_walk uses five_char_walk"]
  ScoreValidator -- "optimize" --> CheckBPMF["check_bpmf_output uses elif by length"]

Loading

File Walkthrough

Relevant files

Bug fix

phrase_deriver.py Replace strict zip for Python 3.9 compatibility                    Source/Data/curation/builders/phrase_deriver.py Remove zip(strict=True) for compatibility. Add comment explaining Python 3.9 support. +2/-1      score_validator.py Fix validator parameters and walker logic                                Source/Data/curation/validators/score_validator.py Pass phrases to seg_pick across cases. Correct walker calls and depths. Switch to elif chain in output checker. Use five_char_walk in six_char path. +27/-26

---

[gemini-code-assist]

Summary of Changes

Hello @tianjianjiang, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses and resolves critical runtime errors and compatibility issues encountered during the Python 3 modernization, which were causing failures in GitHub Actions workflows. The changes primarily involve adjusting function call signatures to pass required arguments and updating zip() usage for broader Python 3 compatibility, ensuring the stability and correctness of data validation processes. All fixes have been verified through syntax validation, full data builds, data integrity checks, and score validator execution.

Highlights

• Python 3 Compatibility: Resolved critical runtime errors and compatibility issues that arose during the Python 3 modernization, specifically addressing failures in GitHub Actions workflows.
• score_validator.py Fixes: Corrected numerous function calls in score_validator.py by adding the missing phrases argument to seg_pick() and three_char_walk() calls, and fixed an incorrect walker function call from four_char_walk to five_char_walk.
• phrase_deriver.py Fixes: Removed the strict=True argument from zip() calls in phrase_deriver.py to ensure compatibility with Python 3.9 and earlier versions, as the strict parameter was introduced in Python 3.10.
• Control Flow Optimization: Optimized the check_bpmf_output() function in score_validator.py by converting multiple independent if statements into an elif chain, preventing redundant execution and improving efficiency.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review API Consistency All new seg_pick calls now pass phrases as the first argument; verify seg_pick's signature expects this order and that no other callers still use the old signature, to avoid mixed calling conventions at runtime. (segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) thisbpmf = "-".join(bpmfinput[2:4]) (a, b) = two_char_walk(phrases, thisbpmf) segcand += a segscore += b candidate.append((segcand, segscore)) # 123-4 if bpmfinput[3] not in XXXXXXX and "-".join(bpmfinput[0:3]) in phrases: segcand = "" segscore = 0 thisbpmf = "-".join(bpmfinput[0:3]) (segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) mybpmf = bpmfinput[3] (segcand, segscore) = seg_pick(phrases, mybpmf, segcand, segscore) candidate.append((segcand, segscore)) # 1234 thisbpmf = "-".join(bpmfinput[0:4]) if thisbpmf in phrases: segcand = "" segscore = 0 (segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) candidate.append((segcand, segscore)) Logic Change In six_char_walk, the recursive call changed from four_char_walk to five_char_walk for the 1-23456 case. Confirm this matches the intended segmentation path and does not alter scoring compared to prior logic. (segcand, segscore) = seg_pick(phrases, mybpmf, segcand, segscore) thisbpmf = "-".join(bpmfinput[1:6]) (a, b) = five_char_walk(phrases, thisbpmf) segcand += a Control Flow check_bpmf_output now uses an elif chain based on computed length; confirm there are no lengths outside 2..6 and consider handling unexpected lengths to avoid silent no-op. length = len(bpmf2chk.split("-")) if length == 2: (a, b) = two_char_walk(phrases, bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") elif length == 3: (a, b) = three_char_walk(phrases, bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") elif length == 4: (a, b) = four_char_walk(phrases, bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") elif length == 5: (a, b) = five_char_walk(phrases, bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") elif length == 6: (a, b) = six_char_walk(phrases, bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}")

[gemini-code-assist]

Code Review

This pull request effectively addresses several runtime errors and compatibility issues that arose during a Python 3 migration. The fixes, which include adding missing function arguments, correcting wrong function calls, and ensuring Python 3.9 compatibility, are all accurate and crucial for resolving the workflow failures. The optimization in check_bpmf_output by switching to an if/elif chain is also a good improvement. I've added one suggestion to further refactor check_bpmf_output to reduce code duplication and improve its maintainability.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restore strict length validation Dropping strict=True removes the length check and can silently truncate pairs when lengths mismatch. Add an explicit length equality guard before zipping to preserve the original correctness. Source/Data/curation/builders/phrase_deriver.py [53-55] # Manual length validation for Python 3.9+ compatibility (strict=True requires 3.10+) +if len(reading_parts) != len(self.value): + return None self._cached_zipped_readings_and_values = list(zip(reading_parts, self.value)) Suggestion importance[1-10]: 7 __ Why: The PR already checks length at line 50, but the suggestion correctly warns about silent truncation when removing strict=True and proposes an explicit guard near the zip. Impact is moderate since the length check exists earlier, making this mostly redundant but still correctness-preserving.	Medium
	Keep seg_pick arguments consistent After changing seg_pick calls to include phrases, verify all subsequent calls within the same branch are updated consistently; a missed update will cause runtime errors. Specifically, ensure any seg_pick(mybpmf, ...) calls also pass phrases. Source/Data/curation/validators/score_validator.py [155] +(segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) - Suggestion importance[1-10]: 6 __ Why: The PR updates seg_pick to accept phrases and this suggestion rightly emphasizes consistency; the shown snippet matches new usage and helps prevent runtime errors, though it mostly reiterates changes already present in the diff.	Low
General	Ensure correct recursive length usage The first branch of six_char_walk should recurse into a five-character walk for the tail only when splitting as 1-23456; your new call is correct, but earlier you changed another similar branch from four to five elsewhere. Ensure all analogous branches use the correct target length. Here, also add a candidate append like other branches to avoid dropping this path if it is missing. Source/Data/curation/validators/score_validator.py [274-276] +thisbpmf = "-".join(bpmfinput[1:6]) +(a, b) = five_char_walk(phrases, thisbpmf) - Suggestion importance[1-10]: 3 __ Why: The provided existing and improved code are identical and the branch already appends the candidate later (line 278), so the suggestion offers minimal change here; the cross-branch audit note is reasonable but vague and not directly actionable for this snippet.	Low