Add CLI args and __main__ guard to final.py (part of #10) - #12
Open
abhishekKokadwar wants to merge 1 commit into
Open
abhishekKokadwar wants to merge 1 commit into
abhishekKokadwar wants to merge 1 commit into
Conversation
final.py previously hardcoded its input/output filenames (annotated_pos_sent.txt, output.vrt) and ran file I/O at import time, inconsistent with every other script in the repo. Added argparse with the same filenames as defaults (backward compatible) and wrapped the logic in main() behind an if __name__ == "__main__" guard, so the module can be imported and unit-tested without side effects. Also carries forward the len(parts) < 10 fix from RedHenLab#11, since this PR branches from main rather than that fix's branch. Addresses the remaining two items from RedHenLab#10; the IndexError fix is in RedHenLab#11.
There was a problem hiding this comment.
Pull request overview
This PR updates the Farsi pipeline’s final.py script to behave like a proper CLI tool (rather than executing file I/O at import time), enabling batch/pipeline usage with arbitrary input/output paths and making the module safe to import for testing.
Changes:
- Added
argparse(-i/--input,-o/--output) with defaults matching the prior hardcoded filenames. - Moved processing and file I/O into
main(input_file, output_file)and added anif __name__ == "__main__":guard. - Added a self-check test script that imports
finaland verifies both the short-line regression and path-based I/O.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| farsi/final.py | Adds CLI args and __main__ guard; wraps processing in main() while preserving existing defaults. |
| farsi/test_final.py | Adds a self-check that imports final and validates short-line handling + custom input/output paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
4
to
6
| def convert_dependency_tree(vrt_content): | ||
| result = [] | ||
| for line in vrt_content.strip().split('\n'): |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| parser = argparse.ArgumentParser(description="Post-process a UDPipe-annotated CONLL-U file into .fa.txt format.") |
Comment on lines
+1
to
+2
| # Self-check for final.py. | ||
| # Run directly: python test_final.py |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the two remaining items from #10 (the IndexError is fixed separately in #11).
Bug
final.pyhardcoded its input/output filenames (annotated_pos_sent.txt,output.vrt) and ran all its logic, including file I/O, at import time with noif __name__ == "__main__":guard. This is inconsistent with every other script inenglish/andfarsi/(e.g.convert_to_xml.pyusesargparse), breaks the batch pipeline usage described inFarsi_readme.mdstep 5 (which expects arbitrary file paths), and means the script can't be imported/unit-tested without side effects.Fix
argparsewith-i/--inputand-o/--output, using the previous hardcoded names as defaults so existing usage (python final.pywith no args, from a directory containingannotated_pos_sent.txt) still works unchanged.main(input_file, output_file)behind anif __name__ == "__main__":guard.Note on overlap with #11
This branch was cut from
main(not from #11's branch), so it also carries forward thelen(parts) < 10guard fix from #11 to keep the file consistent — otherwise this PR would silently regress that fix. If #11 merges first, this will need a trivial rebase (the only overlap is that one line + a whitespace nit); happy to do that once #11 is in.Testing
Added
farsi/test_final.py, which imports the module directly (safe now that there's no import-time I/O) and checks:main()correctly reads from and writes to caller-supplied paths