IBM / cloudant-python-sdk

Cloudant SDK for Python
Apache License 2.0
46 stars 20 forks source link

Inconsitent Typing #527

Closed NielsKorschinsky closed 1 year ago

NielsKorschinsky commented 1 year ago

Describe the bug

Hi,

This is more of a "Flavor" bug report, as I know the current existing syntax is allowed by some linters. However, if using strict typing, the current way the methods are typed causes issues and enforces me to use # type: ignore, which is not preferable, as this will also disable actual type issues.

You are using method headers like:


class ViewQuery: 
    def __init__(
        self,
        *,
        att_encoding_info: bool = None,
        attachments: bool = None,
        conflicts: bool = None,
        descending: bool = None,
        include_docs: bool = None,
        inclusive_end: bool = None,
        limit: int = None,
        skip: int = None,
        update_seq: bool = None,
        end_key: object = None,
        end_key_doc_id: str = None,
        group: bool = None,
        group_level: int = None,
        key: object = None,
        keys: List[object] = None,
        reduce: bool = None,
        stable: bool = None,
        start_key: object = None,
        start_key_doc_id: str = None,
        update: str = None,
    ) -> None:

The issue is, that you have there elements like limit: int = None, This already conflicts, as limit is not set to be an Optional[int]. If using an optional int to access this method, my linter is correctly showing me an issue, as the types conflict.

If used correctly, either the type should be all changed to Optional, or the provided value should be the correct value to disable/default behavior of said argument.

To Reproduce

  1. Use a linter on strict settings.
  2. Try not to use the limit argument, therefore setting it to None
  3. Linter shows error
image image

Expected behavior

The method header is either has the correct types or uses default/disable values which fits the expected types.

Screenshots

Must gather (please complete the following information):

Additional context

NielsKorschinsky commented 1 year ago

I want also to use this issue report to highlight that I really like the fact that typing was applied at all. Also, the way you can interact with the API is very smooth and good to handle. Thanks a lot for providing the API!

emlaver commented 1 year ago

Hi @NielsKorschinsky, thanks for this feedback! Since our SDK is generated from an OpenAPI spec, we don't directly modify several of the files including the CloudantV1 class methods. We will consider passing along your type changes to the team that manages the OpenAPI spec generator for future enhancements.

mojito317 commented 11 months ago

Hi @NielsKorschinsky! FYI: The typing was improved on our latest (v0.7.0) version.