Feature/anonymize various formats#19
Conversation
This commit extends the anonymizer to support Markdown (.md) and plain text (.txt) files in addition to PDFs. The file processing logic has been refactored to be more universal. A file type detection mechanism has been added to select the appropriate text chunker based on the file extension: - `MarkdownTextSplitter` is used for PDF and Markdown files. - `RecursiveCharacterTextSplitter` is used for text files. The core application logic and CLI have been updated to handle generic file paths instead of being limited to PDFs. The output file generation now preserves the original file's extension, except for PDFs, which are converted to Markdown. The README.md has also been updated to reflect these new features and usage instructions.
Summary of ChangesHello @leo-gan, I'm Gemini Code Assist1! 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 significantly extends the functionality of the anonymization application by enabling it to process and anonymize Personally Identifiable Information (PII) not only from PDF documents but also from Markdown and plain text files. This enhancement broadens the tool's applicability across various document formats, ensuring consistent PII replacement and mapping while adapting output formats to match the input where appropriate. The underlying architecture has been refactored to support this multi-format capability, along with updates to the command-line interface, documentation, and testing framework. Highlights
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extends the anonymization feature to support Markdown and text files in addition to PDFs. The changes are well-organized, with core logic refactored for generality and a new module for file loading. The CLI, documentation, and tests have been updated to reflect these new capabilities. I've identified a couple of areas for improvement: one to reduce code duplication for better maintainability, and a bug fix for more robust filename handling during deanonymization.
|
|
||
| file_stem = Path(anonymized_file_path).stem.replace(".anonymized", "") | ||
| anonymized_path = Path(anonymized_file_path) | ||
| file_stem = anonymized_path.name.replace(f".anonymized{anonymized_path.suffix}", "") |
There was a problem hiding this comment.
Using str.replace() to remove the suffix can lead to unexpected behavior if the filename contains multiple instances of .anonymized{ext}. For example, a file named report.anonymized.version.anonymized.md would be incorrectly processed. It's safer to use str.removesuffix() (available in Python 3.9+) to remove only the trailing part of the string.
| file_stem = anonymized_path.name.replace(f".anonymized{anonymized_path.suffix}", "") | |
| file_stem = anonymized_path.name.removesuffix(f".anonymized{anonymized_path.suffix}") |
| elif file_extension == ".txt": | ||
| with open(file_path, "r", encoding="utf-8") as f: | ||
| text = f.read() | ||
| splitter = RecursiveCharacterTextSplitter( | ||
| chunk_size=characters_to_anonymize, chunk_overlap=0 | ||
| ) | ||
| docs = splitter.create_documents([text]) | ||
| return [doc.page_content for doc in docs] | ||
| else: | ||
| logging.warning( | ||
| f"Unsupported file type: {file_extension}. Treating as plain text." | ||
| ) | ||
| with open(file_path, "r", encoding="utf-8") as f: | ||
| text = f.read() | ||
| splitter = RecursiveCharacterTextSplitter( | ||
| chunk_size=characters_to_anonymize, chunk_overlap=0 | ||
| ) | ||
| docs = splitter.create_documents([text]) | ||
| return [doc.page_content for doc in docs] |
There was a problem hiding this comment.
The logic for handling .txt files and other unsupported file types is duplicated. You can combine these blocks to improve maintainability by creating a single else block that handles both cases, logging a warning only for file types that are not .txt.
| elif file_extension == ".txt": | |
| with open(file_path, "r", encoding="utf-8") as f: | |
| text = f.read() | |
| splitter = RecursiveCharacterTextSplitter( | |
| chunk_size=characters_to_anonymize, chunk_overlap=0 | |
| ) | |
| docs = splitter.create_documents([text]) | |
| return [doc.page_content for doc in docs] | |
| else: | |
| logging.warning( | |
| f"Unsupported file type: {file_extension}. Treating as plain text." | |
| ) | |
| with open(file_path, "r", encoding="utf-8") as f: | |
| text = f.read() | |
| splitter = RecursiveCharacterTextSplitter( | |
| chunk_size=characters_to_anonymize, chunk_overlap=0 | |
| ) | |
| docs = splitter.create_documents([text]) | |
| return [doc.page_content for doc in docs] | |
| else: | |
| if file_extension != ".txt": | |
| logging.warning( | |
| f"Unsupported file type: {file_extension}. Treating as plain text." | |
| ) | |
| with open(file_path, "r", encoding="utf-8") as f: | |
| text = f.read() | |
| splitter = RecursiveCharacterTextSplitter( | |
| chunk_size=characters_to_anonymize, chunk_overlap=0 | |
| ) | |
| docs = splitter.create_documents([text]) | |
| return [doc.page_content for doc in docs] |
1 participant
No description provided.