#966 Add sketch typing for utils.py

[butvinm]

I add sketch types for utils.py but have following issues:

Optional imports

OAuth2Credentials, AccessTokenCredentials, GoogleCredentials, ServiceAccountCredentials are optional and only needed for oauth2client, so cannot be resolved as a type.

Currently, I have to use the following code to make it work:

def _convert_oauth(credentials: Any) -> Credentials:
    ...

def _convert_service_account(credentials: Any) -> Credentials:
    ...

But it's not ideal, because it's not type safe. I think, we can add try-except imports to functions body:

def _convert_oauth(credentials: Any) -> Credentials:
    try:
        from oauth2client.client import OAuth2Credentials, AccessTokenCredentials, GoogleCredentials
    except ImportError:
        pass
    ...

NamedTuples used as enums

NamedTuples are used for options, but it's not proper way to use them. It also produce type checking issues.

I think, we can use Enum instead of NamedTuple, if there is no reasons for backward compatibility.

Some unclear types

I'm not sure if I have set the correct types for some functions because they are not explained anywhere. Therefore, I need some validation of my propositions.

[lavigne958]

Hi I'll have a look at the PR.

speaking of the credentials:

• I know it's an issue
• I'll have a look at the code later, we might not need it anymore, we are on the next major release if it's not necessary we should be able to remove that entire function, but first I need to check

speaking of the Namedtuple:

• yes I agree, we can change that, but that will impact other files (like worksheet.py)
• I propose we first type what we can
• In a second PR we makes changes to use Enums instead of NamedTuple

speaking of functions:

• It's a nightmare to type, I managed in the past to type some of the wrappers but using mostly some Any parameters are they are generics and can be anything.
• I'll have a look and make some proposals if I find anything better but I don't mind so much if it's ot perfect

Thank you for this contribution, the major goal here is to move forward and to type what we can so we can have a new 6.0.0 release the sooner the better 🙃

[lavigne958]

Nice work thank you for this.

I have only a few comments regarding some details.

• About the callable, that's fine to me, I might rework it anyway
• could you please format your code, by running: tox -e format in order to make it pass the CI, thanks.

[butvinm]

I updated the PR

[lavigne958]

sorry I was wrong before, you were right about the type for the default_blank it should be as mentioned in the comment. Users are allowed to pass None as value for default blank.

apart from that I see issues with the named tuples but that will go away anyway so no worries bout them.

I see a couple of issues with the float("inf") which has only be a pain to handle so let's merge it like this (after you revert the default blank type) then I'll have a look if I keep the Inf value of not.

thanks again for your work.

[butvinm]

I also changed type of default_blank at numericise method, because it has same behaviour as numericise_all

[lavigne958]

looks good to me.

thank you for your contribution.