feat: modernize Python project

[tianjianjiang]

User description

Modernizes Python codebase to 3.12+ with type hints and clean architecture.

---

PR Type

Enhancement, Documentation

---

Description

• Migrate data tools to Python 3.12+

• Add typing, refactors, better I/O handling

• New encoding audit CLI utility

• Improve CLIs, errors, and Unicode safety

---

Diagram Walkthrough

flowchart LR
  tools["Data tools (compilers/builders/scripts)"] -- "py312 typing, refactors" --> io["Robust I/O & Unicode"]
  io -- "PROJECT_ROOT, encoding=utf-8" --> stability["Stability & portability"]
  tools -- "new CLI" --> audit["audit_encoding.py"]
  compilers["Compilers"] -- "functions split + typing" --> outputs["Deterministic outputs"]

Loading

File Walkthrough

	Relevant files
Enhancement	12 files score_validator.py Py3 refactor with typing and safer I/O                                      +235/-155 main_compiler.py Decompose compiler, add loaders and typing                              +136/-185 analyze_data.py Modular analysis with functions and CLI main                          +199/-127 text_filter.py Regex builder modernization and py3 printing                          +47/-47  frequency_builder.py Typed, modular frequency generation pipeline                          +74/-57  count_occurrences.py Parallel counter with argparse and robust I/O                        +69/-65  phrase_deriver.py Safer zipping, minor cleanup and typing                                    +12/-25  map_bpmf.py Add loaders, PROJECT_ROOT paths, safe mapping                        +53/-55  plain_bpmf_compiler.py Extract loaders, skip pattern, f-strings                                  +44/-28  audit_encoding.py New encoder audit CLI for BPMFBase.txt                                      +86/-0    compiler_utils.py Type hints and stable float formatting                                      +7/-10    playground.ipynb Update cache usage and DataFrame vars                                        +9/-269
Configuration changes	1 files pyproject.toml Require Python 3.12+, add ruff config and CLI                        +16/-6
Documentation	2 files AGENTS.md Update docs for Python 3.12 and paths                                        +4/-4      README.md Refresh data documentation and tooling notes                          +314/-102
Additional files	31 files README +0/-67    Makefile +0/-6      count.bash +0/-19    count.occurrence.c +0/-43    DEPRECATED.md +0/-175  README +0/-238  build.bash +0/-11    filter.bash +0/-41    audit_encoding.swift +0/-83    bpmfmap.py +0/-63    buildFreq.py +0/-73    cook-plain-bpmf.py +0/-44    cook.py +0/-251  cook_util.py +0/-35    count.bash +0/-11    count.occurrence.py +0/-90    derive_associated_phrases.py +0/-118  BIG5toUTF8.pl +0/-20    bpmfmap_human.py +0/-148  build4wlist.bash +0/-5      buildFreq.bash +0/-20    cook.rb +0/-188  count.occurrence.pl +0/-9      countphrase.bash +0/-7      randomShuffle.bash +0/-43    typocorrection.bash +0/-2      utf8length.pl +0/-21    nonCJK_filter.py +0/-67    self-score-test.py +0/-323  README.md +0/-44    requirements.txt +0/-4

---

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1ef9514
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Pass missing arguments to helpers Several helper calls to seg_pick/walkers omit the required phrases argument, causing runtime errors. Ensure every helper invocation passes phrases consistently. This bug also appears in other walkers; fix all similar call sites. Source/Data/curation/validators/score_validator.py [125-176] def four_char_walk( phrases: dict[str, list[tuple[str, float]]], bpmf2walk: str ) -> tuple[str, float]: bpmfinput = bpmf2walk.split("-") candidate = [] # 1-2-3-4, 1-23-4, 1-2-34, 1-234 if ( bpmfinput[0] not in XXXXXXX and bpmfinput[1] not in XXXXXXX and bpmfinput[2] not in XXXXXXX and bpmfinput[3] not in XXXXXXX ): segcand = "" segscore = 0 mybpmf = bpmfinput[0] (segcand, segscore) = seg_pick(phrases, mybpmf, segcand, segscore) thisbpmf = "-".join(bpmfinput[1:4]) (a, b) = three_char_walk(phrases, thisbpmf) segcand += a segscore += b candidate.append((segcand, segscore)) # 12-3-4, 12-34 if ( bpmfinput[2] not in XXXXXXX and bpmfinput[3] not in XXXXXXX and "-".join(bpmfinput[0:2]) in phrases ): segcand = "" segscore = 0 thisbpmf = "-".join(bpmfinput[0:2]) - (segcand, segscore) = seg_pick(thisbpmf, segcand, segscore) + (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(thisbpmf, segcand, segscore) + (segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) mybpmf = bpmfinput[3] - (segcand, segscore) = seg_pick(mybpmf, segcand, segscore) + (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(thisbpmf, segcand, segscore) + (segcand, segscore) = seg_pick(phrases, thisbpmf, segcand, segscore) candidate.append((segcand, segscore)) # candidate.sort(key=lambda x: x[1], reverse=True) return candidate[0] Suggestion importance[1-10]: 8 __ Why: Correct: several calls to seg_pick in four_char_walk omit the required phrases argument (e.g., lines where (segcand, segscore) = seg_pick(thisbpmf, segcand, segscore)), which would raise a TypeError. Fixing these is important for correctness in path exploration.	Medium
	Avoid strict zip to prevent crash Using zip(..., strict=True) will raise a ValueError on Python versions prior to 3.10 or if lengths mismatch elsewhere, causing the builder to crash. Remove strict=True and rely on the earlier explicit length check to keep behavior robust. Source/Data/curation/builders/phrase_deriver.py [53-55] -self._cached_zipped_readings_and_values = list(zip(reading_parts, self.value, strict=True)) +self._cached_zipped_readings_and_values = list(zip(reading_parts, self.value)) return self._cached_zipped_readings_and_values Suggestion importance[1-10]: 7 __ Why: Using strict=True can raise on older Python or mismatched lengths; since a length check already exists, removing strict improves compatibility without changing logic.	Medium
	Fix incorrect README path The readme path should be relative to the project root where pyproject.toml resides. If this file lives in Source/Data/, referencing "README.md" will likely fail packaging metadata rendering on PyPI. Point it to the in-directory README to avoid broken long description. Source/Data/pyproject.toml [9] -readme = "README.md" +readme = "Source/Data/README.md" Suggestion importance[1-10]: 2 __ Why: The existing_code matches line 9, but in PEP 621 the readme path is relative to the directory containing pyproject.toml, so "README.md" is already correct; changing to Source/Data/README.md would be wrong.	Low
General	Align classifiers with tested versions If the codebase isn't CI-tested and guaranteed to work on 3.13, advertising 3.13 support can mislead users. Either ensure CI covers 3.13 or remove the classifier to match actual tested versions. Source/Data/pyproject.toml [21] -"Programming Language :: Python :: 3.13", +# Remove the 3.13 classifier until verified by CI +# "Programming Language :: Python :: 3.13", Suggestion importance[1-10]: 6 __ Why: The line for Python 3.13 exists at 21; advising removal until CI verifies is reasonable quality control, though it's a process check rather than a code fix so impact is moderate.	Low
	Make README reference clickable Confirm the path is correct from repo root. If contributors open this from elsewhere (e.g., PyPI sdist or different CWD), a relative link may break. Use a repository-absolute link to ensure it's always navigable in rendered contexts. AGENTS.md [91] -**For detailed dictionary data documentation**, see `Source/Data/README.md` which covers file formats, editing workflows, Python tools, and troubleshooting. +**For detailed dictionary data documentation**, see [Source/Data/README.md](Source/Data/README.md) which covers file formats, editing workflows, Python tools, and troubleshooting. Suggestion importance[1-10]: 5 __ Why: The existing_code corresponds to line 91; converting an inline code path to a markdown link improves readability and navigation but is a minor documentation enhancement.	Low
	Harden file loading and parsing The signature advertises returning None but the function actually populates module-level dicts; more critically, it lacks error handling and will crash if the file is missing. Wrap file I/O in try/except and early-return on malformed lines to prevent crashes during compilation. Source/Data/curation/validators/score_validator.py [38-56] def load_bpmf_base(file_path: str) -> None: """Load base BPMF character mappings into bpmf_chars and bpmf_phrases.""" - with open(file_path, encoding="utf-8") as f: - for line in f: - if not line or line[0] == "#": - continue - elements = line.rstrip().split() - if len(elements) < 5: - continue - mykey, myvalue = elements[0], elements[1] - if mykey in bpmf_chars: - bpmf_chars[mykey].append(myvalue) - else: - bpmf_chars[mykey] = [myvalue] - if mykey in bpmf_phrases: - bpmf_phrases[mykey].append(myvalue) - else: - bpmf_phrases[mykey] = [myvalue] + try: + with open(file_path, encoding="utf-8") as f: + for line in f: + if not line or line[0] == "#": + continue + elements = line.rstrip().split() + if len(elements) < 2: + continue + mykey, myvalue = elements[0], elements[1] + bpmf_chars.setdefault(mykey, []).append(myvalue) + bpmf_phrases.setdefault(mykey, []).append(myvalue) + except OSError as e: + print(f"Error reading {file_path}: {e}") + sys.exit(1) Suggestion importance[1-10]: 4 __ Why: The proposed robustness is reasonable but targets a different file than specified and partially duplicates patterns already used elsewhere in the PR; impact is moderate and not strictly required for correctness here.	Low

---

Previous suggestions

Suggestions up to commit 5ec4235

Category	Suggestion	Impact
General	Handle missing BPMF mappings Emitting empty strings for unmapped characters will add extra spaces and produce misleading output. Explicitly skip or mark missing mappings to keep output consistent and detectable for downstream tools. Source/Data/scripts/map_bpmf.py [63-69] -pronunciations = [bpmf_mappings.get(char, "") for char in word] +pronunciations = [] +for char in word: + p = bpmf_mappings.get(char) + if p: + pronunciations.append(p) + else: + pronunciations.append("<UNK>") phon = " ".join([word] + pronunciations) print(phon) Suggestion importance[1-10]: 7 __ Why: Emitting placeholders instead of empty strings improves downstream detectability and prevents confusing spacing; the change is accurate and enhances robustness without altering core logic.	Medium
	Remove stray no-op access The standalone elements[4] expression is a no-op and likely a leftover from earlier code. Remove it to avoid confusion and potential linter errors. Keep the length check to guard index access. Source/Data/curation/compilers/main_compiler.py [68-75] def load_bpmf_base(file_path: str) -> None: ... elements = line.rstrip().split() if len(elements) < 5: continue - elements[4] mykey = elements[0] myvalue = elements[1] Suggestion importance[1-10]: 5 __ Why: The standalone elements[4] is indeed a no-op and should be removed; it likely came from earlier code referencing the type field. This is a minor cleanliness fix that avoids linter warnings.	Low
	Avoid unsafe auto-upgrade rules With target-version = "py39", enabling UP (pyupgrade) may suggest changes incompatible with runtime constraints elsewhere. If migration is partial, temporarily exclude UP to avoid automated rewrites that break compatibility. Source/Data/pyproject.toml [78-80] [tool.ruff.lint] -select = ["E", "F", "I", "N", "UP", "B"] +select = ["E", "F", "I", "N", "B"] ignore = ["E501"] Suggestion importance[1-10]: 5 __ Why: The ruff lint config lines (78–80) exist and the advice to drop UP can prevent unintended rewrites; impact is modest and depends on project readiness for pyupgrade.	Low
Possible issue	Prevent multiple branch execution These independent if-blocks will all run when length is 6, causing multiple walks and prints. Use an if/elif chain so only the matching branch executes. This avoids redundant computation and duplicated output. Source/Data/curation/validators/score_validator.py [332-357] def chkBPMFoutput(phrases, bpmf2chk): - if len(bpmf2chk.split("-")) == 2: + parts_len = len(bpmf2chk.split("-")) + if parts_len == 2: (a, b) = twoCharWalk(bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") - if len(bpmf2chk.split("-")) == 3: + elif parts_len == 3: (a, b) = threeCharWalk(bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") - if len(bpmf2chk.split("-")) == 4: + elif parts_len == 4: (a, b) = fourCharWalk(bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") - if len(bpmf2chk.split("-")) == 5: + elif parts_len == 5: (a, b) = fiveCharWalk(bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") - if len(bpmf2chk.split("-")) == 6: + elif parts_len == 6: (a, b) = sixCharWalk(bpmf2chk) (c, d) = phrases[bpmf2chk][0] if (a, b) != (c, d): print(f"{c} {d:f} {a} {b:f}") Suggestion importance[1-10]: 6 __ Why: The existing code uses multiple independent if checks causing redundant work for longer inputs; switching to elif is a correct, low-risk micro-optimization that improves clarity. Impact is moderate since functional correctness isn’t broken today.	Low
	Fix README path for packaging Ensure Source/Data/README.md exists at publish time or set a path relative to this pyproject. If the file actually lives under Source/Data/README.md, point readme to the correct relative path to avoid build failures on packaging. Source/Data/pyproject.toml [9] -readme = "README.md" +readme = "Source/Data/README.md" Suggestion importance[1-10]: 6 __ Why: The snippet readme = "README.md" is at line 9 and is valid TOML, but the suggestion raises a reasonable packaging concern about the correct relative path; impact is moderate and context-dependent.	Low
	Correct console script module path Verify the entry point module path is importable from the package root. If scripts/audit_encoding.py is inside Source/Data/, include the package in packages and use its fully-qualified module path to prevent console_script import errors. Source/Data/pyproject.toml [48] -mcbpmf-audit-encoding = "scripts.audit_encoding:main" +mcbpmf-audit-encoding = "curation.scripts.audit_encoding:main" Suggestion importance[1-10]: 4 __ Why: The entry point line exists at 48, but changing it to curation.scripts.audit_encoding is speculative without proof of package structure; it's a valid verification note with uncertain necessity.	Low

[github-actions]

Persistent review updated to latest commit 1ef9514

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Build

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #719 at branch `refactor/py3_migration`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[github-actions]

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

• Check the workflow logs for details
• Trigger a manual review by commenting @claude on this PR
• The review will be retried automatically on the next push

This does not affect the PR approval process.