add data extraction tutorial - #198
Conversation
Reviewer's GuideThis PR adds three comprehensive tutorial notebooks that guide users through an end-to-end scientific data extraction workflow with LLMs, implementing scaffolding and examples for preprocessing, model interfacing, and postprocessing/evaluation. Class diagram for structured extraction schema (Pydantic models)classDiagram
class PolymerExtraction {
+str name
+Optional~str~ synthesis_method
+Optional~List~ temperature
+Optional~bool~ homopolymer
}
class PolymerList {
+List~PolymerExtraction~ polymers
}
PolymerList --> PolymerExtraction : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @marawilhelmi - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The call to the chunking function is missing. (link)
General comments:
- Remove or implement all TODO placeholders so the tutorial notebooks are runnable and self-contained rather than leaving stub variables in key code cells.
- Update the notebook metadata to ensure the kernel and language_info versions accurately reflect the Python environment (e.g. use Python 3.x everywhere instead of mixing with 2.7).
- Consider refactoring repeated file I/O patterns (listing PDFs, reading/writing text files) into shared helper functions to reduce duplication across the notebooks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove or implement all TODO placeholders so the tutorial notebooks are runnable and self-contained rather than leaving stub variables in key code cells.
- Update the notebook metadata to ensure the kernel and language_info versions accurately reflect the Python environment (e.g. use Python 3.x everywhere instead of mixing with 2.7).
- Consider refactoring repeated file I/O patterns (listing PDFs, reading/writing text files) into shared helper functions to reduce duplication across the notebooks.
## Individual Comments
### Comment 1
<location> `tutorial/data_extraction_tutorial_1.ipynb:162` </location>
<code_context>
+ "import os\n",
+ "import fitz # PyMuPDF\n",
+ "\n",
+ "pdf_folder = ___ # TODO: Specify the folder path where the PDF files are located\n",
+ "\n",
+ "output_folder = ___ # TODO: Specify the output folder path for the text files\n",
+ "\n",
+ "# Make sure the output folder exists\n",
</code_context>
<issue_to_address>
The folder paths for input and output are left as placeholders.
Please provide example paths or clarify the expected directory structure to help users avoid errors and improve usability.
</issue_to_address>
### Comment 2
<location> `tutorial/data_extraction_tutorial_1.ipynb:180` </location>
<code_context>
+ " # Extract text from all pages\n",
+ " full_text = \"\"\n",
+ " for page in doc:\n",
+ " # TODO Use .get_text() to extract the text from the documents and add it to full_text\n",
+ " \n",
+ " # TODO print the extracted text\n",
</code_context>
<issue_to_address>
The extraction of text from PDF pages is not implemented.
Add a call to .get_text() for each page to populate full_text and ensure the output files contain the extracted text.
</issue_to_address>
### Comment 3
<location> `tutorial/data_extraction_tutorial_1.ipynb:234` </location>
<code_context>
+ " filtered = matches[0].replace(\"Acknowledgements\", \"\")\n",
+ " else:\n",
+ " # If pattern not found, keep original text\n",
+ " # TODO: define the filtered text when no pattern is found \n",
+ " \n",
+ " return filtered"
</code_context>
<issue_to_address>
The fallback for when the regex pattern is not found is not defined.
Assigning filtered = text in the else branch will ensure the function always returns a value and avoids undefined variable errors.
</issue_to_address>
### Comment 4
<location> `tutorial/data_extraction_tutorial_1.ipynb:263` </location>
<code_context>
+ " content = f.read()\n",
+ " \n",
+ " # Clean the content\n",
+ " cleaned = # TODO: add the correct call of the cleaning function\n",
+ "\n",
+ " # TODO: Print the cleaned content\n",
</code_context>
<issue_to_address>
The call to the cleaning function is missing.
This will cause a runtime error since 'cleaned' is not defined. Please call the cleaning function with the correct argument.
</issue_to_address>
### Comment 5
<location> `tutorial/data_extraction_tutorial_1.ipynb:304` </location>
<code_context>
+ " Splits a given text into overlapping chunks using RecursiveCharacterTextSplitter.\n",
+ " \"\"\"\n",
+ " text_splitter = RecursiveCharacterTextSplitter(\n",
+ " chunk_size=___, # TODO: Define chunk size\n",
+ " chunk_overlap=___ # TODO: Define overlap size\n",
+ " )\n",
+ " \n",
</code_context>
<issue_to_address>
Chunk size and overlap are left as placeholders in the text splitter.
Without these values, the chunking function will not operate. Please set default values (e.g., chunk_size=1000, chunk_overlap=200) to ensure functionality.
</issue_to_address>
### Comment 6
<location> `tutorial/data_extraction_tutorial_1.ipynb:329` </location>
<code_context>
+ " with open(os.path.join(cleaned_folder, filename), \"r\", encoding=\"utf-8\") as f:\n",
+ " content = f.read()\n",
+ "\n",
+ " chunks = # TODO: call the chunking function with the content\n",
+ "\n",
+ " print(f\"{len(chunks)} chunks created for {filename}.\")"
</code_context>
<issue_to_address>
The call to the chunking function is missing.
This will cause a runtime error since 'chunks' is undefined. Please call the chunking function with the file content.
</issue_to_address>
### Comment 7
<location> `tutorial/data_extraction_tutorial_2.ipynb:97` </location>
<code_context>
+ "user_prompt = f\"Extract all polymer names from the following text:\\n\\n{document_text}\"\n",
+ "\n",
+ "# Call the model\n",
+ "response = call_llm(___, ___) # TODO: add the prompts to call the call_llm function\n",
+ "\n",
+ "print(response)"
</code_context>
<issue_to_address>
The call to call_llm is missing the required prompt arguments.
This will cause a runtime error; please pass both system and user prompts to call_llm.
</issue_to_address>
### Comment 8
<location> `tutorial/data_extraction_tutorial_3.ipynb:219` </location>
<code_context>
+ "from difflib import SequenceMatcher\n",
+ "\n",
+ "# Define a fuzzy matching function using character similarity\n",
+ "def fuzzy_match(a, b, threshold=___): # TODO: Try out different similarity thresholds\n",
+ " \"\"\"\n",
+ " Returns True if strings `a` and `b` are similar above a certain threshold.\n",
</code_context>
<issue_to_address>
The default threshold for fuzzy matching is left as a placeholder.
A missing threshold will raise a runtime error. Please set a default value, such as 0.8, to avoid this issue.
</issue_to_address>
### Comment 9
<location> `tutorial/data_extraction_tutorial_3.ipynb:240` </location>
<code_context>
+ "for gt in ground_truth:\n",
+ " for pred in predicted:\n",
+ " if fuzzy_match(gt, pred, threshold=0.8): # <-- try different thresholds!\n",
+ " # TODO: Print the match pair\n",
+ " print(f\"✓ '{___}' ↔ '{___}'\")\n"
+ ],
</code_context>
<issue_to_address>
The print statement for matched pairs is left as a placeholder.
Please update the print statement to display the actual matched ground truth and predicted values for complete output.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "pdf_folder = ___ # TODO: Specify the folder path where the PDF files are located\n", | ||
| "\n", | ||
| "output_folder = ___ # TODO: Specify the output folder path for the text files\n", |
There was a problem hiding this comment.
issue: The folder paths for input and output are left as placeholders.
Please provide example paths or clarify the expected directory structure to help users avoid errors and improve usability.
| " # Extract text from all pages\n", | ||
| " full_text = \"\"\n", | ||
| " for page in doc:\n", | ||
| " # TODO Use .get_text() to extract the text from the documents and add it to full_text\n", |
There was a problem hiding this comment.
issue (bug_risk): The extraction of text from PDF pages is not implemented.
Add a call to .get_text() for each page to populate full_text and ensure the output files contain the extracted text.
| " filtered = matches[0].replace(\"Acknowledgements\", \"\")\n", | ||
| " else:\n", | ||
| " # If pattern not found, keep original text\n", | ||
| " # TODO: define the filtered text when no pattern is found \n", |
There was a problem hiding this comment.
issue (bug_risk): The fallback for when the regex pattern is not found is not defined.
Assigning filtered = text in the else branch will ensure the function always returns a value and avoids undefined variable errors.
| " content = f.read()\n", | ||
| " \n", | ||
| " # Clean the content\n", | ||
| " cleaned = # TODO: add the correct call of the cleaning function\n", |
There was a problem hiding this comment.
issue (bug_risk): The call to the cleaning function is missing.
This will cause a runtime error since 'cleaned' is not defined. Please call the cleaning function with the correct argument.
| " chunk_size=___, # TODO: Define chunk size\n", | ||
| " chunk_overlap=___ # TODO: Define overlap size\n", |
There was a problem hiding this comment.
issue (bug_risk): Chunk size and overlap are left as placeholders in the text splitter.
Without these values, the chunking function will not operate. Please set default values (e.g., chunk_size=1000, chunk_overlap=200) to ensure functionality.
| " with open(os.path.join(cleaned_folder, filename), \"r\", encoding=\"utf-8\") as f:\n", | ||
| " content = f.read()\n", | ||
| "\n", | ||
| " chunks = # TODO: call the chunking function with the content\n", |
There was a problem hiding this comment.
issue (bug_risk): The call to the chunking function is missing.
This will cause a runtime error since 'chunks' is undefined. Please call the chunking function with the file content.
| "user_prompt = f\"Extract all polymer names from the following text:\\n\\n{document_text}\"\n", | ||
| "\n", | ||
| "# Call the model\n", | ||
| "response = call_llm(___, ___) # TODO: add the prompts to call the call_llm function\n", |
There was a problem hiding this comment.
issue (bug_risk): The call to call_llm is missing the required prompt arguments.
This will cause a runtime error; please pass both system and user prompts to call_llm.
| "from difflib import SequenceMatcher\n", | ||
| "\n", | ||
| "# Define a fuzzy matching function using character similarity\n", | ||
| "def fuzzy_match(a, b, threshold=___): # TODO: Try out different similarity thresholds\n", |
There was a problem hiding this comment.
issue (bug_risk): The default threshold for fuzzy matching is left as a placeholder.
A missing threshold will raise a runtime error. Please set a default value, such as 0.8, to avoid this issue.
| "for gt in ground_truth:\n", | ||
| " for pred in predicted:\n", | ||
| " if fuzzy_match(gt, pred, threshold=0.8): # <-- try different thresholds!\n", | ||
| " # TODO: Print the match pair\n", |
There was a problem hiding this comment.
issue: The print statement for matched pairs is left as a placeholder.
Please update the print statement to display the actual matched ground truth and predicted values for complete output.
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
"Therefore, you could download a dataset manually or use a tool to help to automate and speed up this process."
"One of these libraries is crossrefapi."
Reply via ReviewNB
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
The next step is to download the relevant papers. There are multiple datasets and tools available which can be used for data mining. While downloading papers, please always be aware of copyright.
Reply via ReviewNB
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
Would you like to explain what tokens are? I think they were not introduced until this point. Or perhaps you can link to a blog explaining them, e.g, https://www.geeksforgeeks.org/nlp/tokenization-in-natural-language-processing-nlp
Reply via ReviewNB
| @@ -0,0 +1,210 @@ | |||
| { | |||
There was a problem hiding this comment.
Above might be specially useful to link to something that explains the differences especially if here you ask them to fill the different prompts
Reply via ReviewNB
| @@ -0,0 +1,374 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
perhaps I would also emphasize where the pydantic model comes in and what the retries do
| @@ -0,0 +1,374 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,374 @@ | |||
| { | |||
There was a problem hiding this comment.
Do you want to provide a link to the pint docs in case someone wants to use it even if it is not for data extraction?
Reply via ReviewNB
| @@ -0,0 +1,374 @@ | |||
| { | |||
There was a problem hiding this comment.
|
So my biggest concern is about the time. You said that you have 3 hours for this which I think might go long. I do not know how do you want to structure it but perhaps propose some exercises, in case someone finishes going through the notebooks, at least it is occupied. Another minor comment is that there is one test failing 😓 |
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
perhaps also add a note how this tutorial works. That there are parts of codes that still need to be filled in and that those are indicated with _
Reply via ReviewNB
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,396 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,374 @@ | |||
| { | |||
There was a problem hiding this comment.
I do not know if this makes the purpose of matching really clear.
Matching is required because you might have k entries in the ground truth and the model will extract n. Out of those n some might be hallucinated, wrong, the model might also have missed some. Hence, it is non-trivial to construct a mapping of what of the k entries to compare to with the n entries.
Reply via ReviewNB
|
@marawilhelmi you did not want to merge this, right? |
3 participants
Summary by Sourcery
Add a three-part hands-on tutorial for scientific data extraction using LLMs, covering data collection and preprocessing, prompt engineering and model integration, and postprocessing with validation and evaluation.
New Features: