Skip to content
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

ensure client functionality is aligned with docs usage #452

Merged
merged 4 commits into from
Aug 29, 2023

Conversation

tsmith023
Copy link
Contributor

@tsmith023 tsmith023 commented Aug 29, 2023

This PR fixes problems surrounding passing lists to value<> fields and is an artefact of the previous confusions surrounding how data types must be passed to Weaviate through the GraphQL and REST functionalities.

In the previous release, the hard requirement was added to the GraphQL filters when this was neither necessary nor required. This PR undoes a final aspect that was missed from the latest patch and is causing confusing errors upon usage due to the difference between the available library and the valid documentation.

A user reported the following problem in the community Slack:

Hi all, what is the correct way to use the new ContainsAll/Any operator in the python client when filtering ? I see there have been some patches in the last few days but I can’t seem to make it work. I’m currently building up a list of operands with my containsAll one looking like:

operands.append(
    {
        "operator": "ContainsAll",
        "valueText": [tag],
        "path": ["tags"],
    }
)

But this always gives me the error Cannot provide a list when constructing where filter for valueText with ['myTag'] , however from what I can see in the docs for containsAll/Any this is the recommended structure. Any help would be appreciated.

The problem is these conditions that I added originally when I thought it was a requirement that the argument value<> match the value when making a where filter. This is only true of batch.delete_objects and not of gql.filter so it is leading to bugs since the docs report that value<> is used with an array argument [] , e.g. "valueInt": [1] etc.

So it will no longer throw a client error when users say "valueText": [tag], only throwing an error if they say "valueTextArray": tag, since that is demonstrably incorrect whereas the first is only semantically incorrect (it still works with Weaviate despite having confusing wording).

@tsmith023 tsmith023 requested a review from dirkkul August 29, 2023 11:45
@dirkkul
Copy link
Collaborator

dirkkul commented Aug 29, 2023

ok, I'm confused now with what works and what doesn't?

Could you please write a few examples that describe exactly where the problem is?

@dirkkul
Copy link
Collaborator

dirkkul commented Aug 29, 2023

to be sure that these are working in the future, can you please add some integration tests with all of these filters?

best would be a parametrized test that goes through each filter/type value type combination

@dirkkul dirkkul merged commit 54053fb into main Aug 29, 2023
19 checks passed
@dirkkul dirkkul deleted the hotfix-where-filtering branch August 29, 2023 13:38
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