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

Incorrect type annotation(s) in AsyncTransaction constructor #537

Closed
anna-hope opened this issue Feb 25, 2022 · 4 comments
Closed

Incorrect type annotation(s) in AsyncTransaction constructor #537

anna-hope opened this issue Feb 25, 2022 · 4 comments
Assignees
Labels
api: firestore Issues related to the googleapis/python-firestore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Milestone

Comments

@anna-hope
Copy link

anna-hope commented Feb 25, 2022

The type annotation for client in AsyncTransaction constructor should be ...async_client.AsyncClient, not ...client.Client:

client (:class:`~google.cloud.firestore_v1.client.Client`):

I can submit a PR to fix this.

I am not sure if the annotation for _commit_with_retry

client: Client, write_pbs: list, transaction_id: bytes
should also refer to AsyncClient. If it should, I can fix that too.

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Feb 25, 2022
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Feb 26, 2022
@kolea2 kolea2 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 3, 2022
@yoshi-automation yoshi-automation removed triage me I really want to be triaged. 🚨 This issue needs some love. labels Mar 3, 2022
@t-kozak
Copy link

t-kozak commented Mar 15, 2022

The type annotations in general are problematic throughout the API. For instance, the AsyncQuery#get method:

async def get(
        self,
        transaction: Transaction = None,
        retry: retries.Retry = gapic_v1.method.DEFAULT,
        timeout: float = None,
    ) -> list:

the transaction here is not market as Optional[Transaction], which causes type checker configured to strict to complain that None is invalid value.

Additionally, in comment of this method:

transaction
                (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]):

the transaction is supposed to be an Optional[Transaction], but not an AsyncTransaction - when you use AsyncClient, you get AsyncTransaction.

As a general note, given the popularity of VisualStudio Code, and thus pylance type checker, it would be awesome if you folks could allocate a day or two of someone's work life to just go through the APIs documentation and annotation and double check that simple use of the library does not involve frequent use of # type:ignore.

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jun 15, 2022
@Mariatta
Copy link
Contributor

Indeed there are mistyped annotations that should be fixed.
And at the moment there are 168 uses of # type: ignore so this will take some time to address.

@Mariatta Mariatta added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 14, 2023
@meredithslota
Copy link
Contributor

Related to #447

@daniel-sanche daniel-sanche added this to the Q2 milestone Feb 23, 2024
@kolea2 kolea2 assigned daniel-sanche and unassigned daniel-sanche Apr 1, 2024
@daniel-sanche
Copy link
Contributor

fixed in #984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

7 participants