Skip to content

Ez3k4 A4#58

Open
Ez3k4 wants to merge 1 commit into
mainfrom
Ez3k4-A4
Open

Ez3k4 A4#58
Ez3k4 wants to merge 1 commit into
mainfrom
Ez3k4-A4

Conversation

@Ez3k4

@Ez3k4 Ez3k4 commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Please review :)

Co-authored-by: Copilot <copilot@github.com>
@julia-gro

julia-gro commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Hi! I really like your README, it has a nice clear structure and is easily understandable. Also I enjoyed your use of argparse. Also your code has a nice and traceable structure and i like that you added the option to set the maximum number of generated tokens.
couple of small issues:

  • i think you forgot to remove line 21 (print(start,suffix))- I suppose youve used it for debugging, but with in it the output is not really usable.
  • your code doesnt recognize all letters of the german keyboard. i had the same issue but i was able to solve this problem by using with open(positional[0], "r", encoding="utf-8") as f:
  • argparse requires inputfile as a positional argument, so piping input doesnt work. maybe you could change inputfile to an optional argument and read from sys.stdin as fallback when none is provided.

Overall a solid and well-structured implementation, I especially liked the suffix array approach! :)

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.

2 participants