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

[Bug] Use Mapping instead of Dict for read-only arguments #305

Open
2 tasks done
Thyrst opened this issue Feb 6, 2024 · 2 comments
Open
2 tasks done

[Bug] Use Mapping instead of Dict for read-only arguments #305

Thyrst opened this issue Feb 6, 2024 · 2 comments
Labels
bug Something isn't working status:backlog This issue has been added to our backlog

Comments

@Thyrst
Copy link

Thyrst commented Feb 6, 2024

Is this a new bug in the Pinecone Python client?

  • I believe this is a new bug in the Pinecone Python Client
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

I query Pinecone index like this:

search_filter = {
    "username": "thyrst",
}
result = pinecone_index.query(
    vector=vector,
    top_k=10,
    filter=search_filter,
    includeMetadata=True,
)

A static type checker fails because of filter argument type.

Type "dict[str, str]" cannot be assigned to type "Dict[str, str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]] | None"
"dict[str, str]" is incompatible with "Dict[str, str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]]

Expected Behavior

The dictionary passed in the filter argument is not changed in the query function, therefore we could use covariant Mapping type instead of Dict, so the type checker doesn't throw.

Steps To Reproduce

  1. Setup Python3 environment
  2. Install dependecies: pip install pinecone-client pyright
  3. Create main.py:
import os
from pinecone import Pinecone


pinecone = Pinecone(api_key=os.environ["PINECONE_API_KEY"])
pinecone_index = pinecone.Index(name=os.environ["PINECONE_INDEX_NAME"])
assert pinecone_index is not None


vector = [0.1, 0.2, 0.3, 0.4, 0.5]  # sample embedding vector
search_filter = {
    "username": "thyrst",
}
result = pinecone_index.query(
    vector=vector,
    top_k=10,
    filter=search_filter,
    includeMetadata=True,
)

print(result)
  1. Run Pyright: pyright main.py

Relevant log output

% pyright main.py
/Users/****/main.py
  /Users/****/main.py:17:12 - error: Argument of type "dict[str, str]" cannot be assigned to parameter "filter" of type "Dict[str, str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]] | None" in function "query"
    Type "dict[str, str]" cannot be assigned to type "Dict[str, str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]] | None"
      "dict[str, str]" is incompatible with "Dict[str, str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]]"
        Type parameter "_VT@dict" is invariant, but "str" is not the same as "str | float | int | bool | List[Unknown] | dict[Unknown, Unknown]"
        Consider switching from "dict" to "Mapping" which is covariant in the value type
      "dict[str, str]" is incompatible with "None" (reportArgumentType)
1 error, 0 warnings, 0 informations

Environment

- OS: macOS Ventura 13.6.2
- Python: 3.11.5
- pinecone-client: 3.0.2
- pyright: 1.1.350

Additional Context

I think covariant types should be used preferably for the input arguments everywhere where the passed objects are not changed.

@Thyrst Thyrst added the bug Something isn't working label Feb 6, 2024
@Thyrst Thyrst changed the title [Bug] Using Mapping instead of Dict for read-only arguments [Bug] Use Mapping instead of Dict for read-only arguments Feb 6, 2024
@Thyrst
Copy link
Author

Thyrst commented Feb 6, 2024

Created a PR with the idea.

@charles-dyfis-net
Copy link

(Folks who want more background on why this is necessary might review https://stackoverflow.com/questions/73603289/why-doesnt-parameter-type-dictstr-unionstr-int-accept-value-of-type-di)

@anawishnoff anawishnoff added status:needs-triage An issue that needs to be triaged by the Pinecone team status:backlog This issue has been added to our backlog and removed status:needs-triage An issue that needs to be triaged by the Pinecone team labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status:backlog This issue has been added to our backlog
Projects
None yet
Development

No branches or pull requests

3 participants