+
Skip to content

Conversation

karanataryn
Copy link
Contributor

@karanataryn karanataryn commented Oct 15, 2024

Fixes bugs in unflattening data and comparing documents and the arising broken tests and bugs in various connectors.

@karanataryn karanataryn requested a review from HenryL27 October 18, 2024 07:14
Comment on lines +156 to +159
try:
key = int(part)
except ValueError:
key = part
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could prob do if key.isnumeric()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part will also be a str though? So I would have to cast it to make it work I think.

Copy link
Collaborator

@HenryL27 HenryL27 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnumeric is a string method to tell you if all the characters are digits. so you could do

if part.isnumeric():
    key = int(part)
else:
    key = part

I just like to avoid extraneous try/catches where possible. maybe that's silly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense. I can do that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait isdigit does the same thing. Why did we change this from the prev implementation?

Copy link
Contributor Author

@karanataryn karanataryn Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because both isdigit and (I just realized) isnumeric don't handle decimals or negative numbers. I'll stick with the current implementation for now.

Copy link
Collaborator

@HenryL27 HenryL27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@karanataryn karanataryn merged commit 79818f5 into main Oct 18, 2024
12 of 13 checks passed
@karanataryn karanataryn deleted the ksampath/fix-weaviate branch October 18, 2024 22:06
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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载