-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update base.py #17269
base: main
Are you sure you want to change the base?
Update base.py #17269
Conversation
Changed the variable "extra_info" to "metadata" for more clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey, while i agree with you that metadata
seems to be a more natural name, i am not sure if its worth changing as it'll be breakign
@@ -91,7 +91,7 @@ def __init__( | |||
self.clean_json = clean_json | |||
|
|||
def load_data( | |||
self, input_file: str, extra_info: Optional[Dict] = {} | |||
self, input_file: str, metadata: Optional[Dict] = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is technically a breaking change?
Hey Andrei,
You're right, I realized that later.
However there is another part that needs to be fixed. Azure Cosmos DB for
No SQL can't take a date object and while running
VectorStoreIndex.from_docuements, a timestamp field is being added
automatically which causes an error. I had to change the cosmos db source
code to handle the datetime object. I was wondering if this can be fixed.
Also I had a question on the entire integration with Cosmos DB NoSQL, is
the integration complete? Because I run into errors so frequently. When I
follow the documentation, and run the query_engine.query("Query String"), I
get a syntax error raised by cosmos db.
It's just been really frustrating because the docs aren't being that
helpful, there are different variations and versions of the docs, its just
really confusing.
Best,
Chinmay
…On Sun, Dec 15, 2024 at 5:41 AM Logan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
llama-index-integrations/readers/llama-index-readers-json/llama_index/readers/json/base.py
<#17269 (comment)>
:
> @@ -91,7 +91,7 @@ def __init__(
self.clean_json = clean_json
def load_data(
- self, input_file: str, extra_info: Optional[Dict] = {}
+ self, input_file: str, metadata: Optional[Dict] = {}
this is technically a breaking change?
—
Reply to this email directly, view it on GitHub
<#17269 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AT5AMHU2JDKSSJNHEZXDV2T2FTCMBAVCNFSM6AAAAABTR7M5WGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMBUGI3TENZZGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@ChinmayR1202 not sure what you mean by different variations/versions of the docs. There is only one docs page for cosmos (but of course, microsoft has two versions of this for some reason 😅 ) My guess is the integration hasn't been used in a while, and is likely out of date with current versions of the cosmos db syntax? Just a guess though |
Changed the variable "extra_info" to "metadata" for more clarity.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
New Package?
Did I fill in the
tool.llamahub
section in thepyproject.toml
and provide a detailed README.md for my new integration or package?Version Bump?
Did I bump the version in the
pyproject.toml
file of the package I am updating? (Except for thellama-index-core
package)Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Your pull-request will likely not be merged unless it is covered by some form of impactful unit testing.
Suggested Checklist:
make format; make lint
to appease the lint gods