Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Thanks for adding this.
I've done the rebasing. |
| @@ -0,0 +1,1075 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Fixed NotFound error
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Moved imports to the top
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #4. from langchain_core.documents import Document
Move the imports to the top.
Reply via ReviewNB
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #4. from langchain_google_vertexai import ChatVertexAI, VertexAIEmbeddings
Move the imports to the top.
Reply via ReviewNB
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
I think it's clearer to split this cell into multiple cells and reorder a bit, splitting the definition and execution.
Reply via ReviewNB
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #4. from langchain_google_vertexai import VertexAIEmbeddings
Move the imports to the top.
Reply via ReviewNB
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #43. print(textwrap.fill(answer, width=80))
On my end the retrieval and output didn't pick up the battery and the answer wasn't very different from conventional RAG. Better to use another query?
Reply via ReviewNB
There was a problem hiding this comment.
This dataset is currently too small to show meaningful improvements over vanilla RAG. We might swap this out for a more complex dataset later
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #5. from langchain_text_splitters import RecursiveCharacterTextSplitter
Move the imports to the top.
Reply via ReviewNB
| @@ -0,0 +1,1091 @@ | |||
| { | |||
There was a problem hiding this comment.
Overall, I think it's better to add more Markdown explanations for each cell, clarifying each step.
Reply via ReviewNB
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
Description is not a complete sentence.
And the cell below is just a download, not adding graph document.
Reply via ReviewNB
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
Add detailed description. This should be a very important cell.
Probably better to split it into multiple cells, and explain each operation one by one.
Reply via ReviewNB
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
It's saying THIS COULD REMOVE DATA FROM YOUR DATABASE !!! but what exactly the reader need to be careful?
What are the previous iterations?
Reply via ReviewNB
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
Describe what readers should take a look at in the visualization. Describe the instruction more explicitly.
Reply via ReviewNB
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,1129 @@ | |||
| { | |||
There was a problem hiding this comment.
Could you not use header syntax (#) for sentence? Please write title for each section, and use plain text for descriptions.
Reply via ReviewNB
|
Code looks good, but please write more detailed descriptions for each cell and operation. And please organize chapters with header syntax (#) properly. The current toc isn't easily understandable. |
2 participants
Added Graph RAG demo solution