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

Issue with Type Hints for fields Parameter in Elasticsearch Python Client #2436

Open
mpaluch92 opened this issue Feb 12, 2024 · 1 comment
Open

Comments

@mpaluch92
Copy link

Elasticsearch version: 8.12.0

elasticsearch-py version: 8.12.0

Description of the problem including expected versus actual behavior:

I am encountering a type mismatch error when attempting to use the Elasticsearch.search method with the fields parameter to retrieve a subset of stored fields from matched documents. According to the Elasticsearch documentation, passing a list of field names as strings to the fields parameter should be sufficient for this purpose Elasticsearch Fields Request.

However, when I specify the fields parameter as a list of strings, both Pylance and PyCharm raise type errors indicating that the expected type is t.Optional[t.Sequence[t.Mapping[str, t.Any]]], rather than t.Optional[t.Union[str, t.Sequence[str]]]. This discrepancy seems inconsistent with the type hints used in other methods like search_mvt or knn_search, where the fields parameter accepts a union of string or sequence of strings.

Despite the type checking errors reported by the IDEs, the actual execution of the search query does yield the expected results, with the returned hits including the requested fields. It appears that the type hints may need updating to reflect the correct usage of the fields parameter for the search method.

Steps to reproduce:

    from elasticsearch import Elasticsearch

    es_instance = Elasticsearch("http://localhost:9200")
    query = {"match": {"title": {"query": "Framework Laptop"}}}
    fields = ["title", "content"]
    es_instance.search(index="products", query=query, fields=fields)

In this snippet, fields is passed as a list of strings, but the type hint suggests that a sequence of mappings is expected. This leads to the type error in Pylance and PyCharm:

Screenshot 2024-02-12 at 11 34 17

Could the type hint for the fields parameter in the search method be updated to align with the expected usage, or is there another way to address this issue without changing the type hint? Am I misunderstanding how to use the fields parameter?

@pquentin
Copy link
Member

Hey @mpaluch92, thank you for your detailed bug report. You are right that passing a list of string is supported and that the type annotation is wrong. In fact, this was initially reported by @JannKleen in #2347, and I forgot to reopen the issue when I realized my fix had to be reverted.

The rest of this comment gives more technical details, but you can ignore them. I'll close this issue when I fix the bug.


The code that is wrong is generated from https://github.com/elastic/elasticsearch-specification. For mvt_search and the deprecated knn_search API, fields is typed as Fields, which boils down to a string or an array or strings. However, this is wrong, because those API also accept objects that define field but also format. For this reason, in the _search API, fields is typed as Array<FieldAndFormat>, with FieldAndFormat being a class. This seemed wrong, so I opened a pull request to change the type to Array<Field | FieldAndFormat>, but I had to revert it because the bug was in the code generator that was not taking into account shortcut_property instead to allow strings, not only mappings.

Long story short, I need to fix the (private) Python code generator to take into account shortcut_property and fix the type hints. In the meantime, you may need # pyright: ignore or # mypy: ignore annotations, sorry.

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

No branches or pull requests

2 participants