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

Mypy errors on some exception in 1.59.0 #2589

Closed
sodul opened this issue Jun 30, 2023 · 8 comments · Fixed by #2591
Closed

Mypy errors on some exception in 1.59.0 #2589

sodul opened this issue Jun 30, 2023 · 8 comments · Fixed by #2591

Comments

@sodul
Copy link

sodul commented Jun 30, 2023

After upgrading to 1.59.0 we got a mypy error on our code catching UnknownObjectException:

    except github.UnknownObjectException as err:
        message = err.data['message']

Now mypy is raising:

error: Invalid index type "str" for "str | dict[str, str | list[str] | list[dict[str, str]]]"; expected type "SupportsIndex | slice"  [index]

This seems to be caused by the improved type annotation of the GithubException base class introduced in #2463:

    @property
    def data(
        self,
    ) -> Union[str, Dict[str, Union[str, List[str], List[Dict[str, str]]]]]:

Also declared in the __init__() signature.

AFAIK UnknownObjectException is only raised by Requester.py always returning data as output: Dict[str, Any] so UnknownObjectException could be updated to clarify this more accurately so that mypy will not raise typing errors that cannot occur at runtime.

@EnricoMi
Copy link
Collaborator

@trim21 can you take a look, please?

@trim21
Copy link
Contributor

trim21 commented Jun 30, 2023

AFAIK UnknownObjectException is only raised by Requester.py always returning data as output: Dict[str, Any] so UnknownObjectException could be updated to clarify this more accurately so that mypy will not raise typing errors that cannot occur at runtime.

Yes, I think you are right about this.

@Borda
Copy link
Contributor

Borda commented Jul 4, 2023

In this moce, may I ask what was the motivation for moving typing to pyi files when the min supported python is 3.7 and it effectively doubles the number of files in the package (and personally, it makes it harder to read as all nested in a single folder)

@trim21
Copy link
Contributor

trim21 commented Jul 4, 2023

In this moce, may I ask what was the motivation for moving typing to pyi files when the min supported python is 3.7 and it effectively doubles the number of files in the package (and personally, it makes it harder to read as all nested in a single folder)

because pyi are added before pygithub drop previous python version, they are essential at that time.

@Borda
Copy link
Contributor

Borda commented Jul 4, 2023

docs suggest it was added recently in 1.59 release: https://github.com/PyGithub/PyGithub/blob/main/doc/changes.rst#version-1590-june-22-2023

@trim21
Copy link
Contributor

trim21 commented Jul 4, 2023

@trim21
Copy link
Contributor

trim21 commented Jul 4, 2023

pyi are removed, inline types are added in 1.59 release

@Borda
Copy link
Contributor

Borda commented Jul 4, 2023

pyi are removed, inline types are added in 1.59 release

you are right, was reading it wrong, 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

Successfully merging a pull request may close this issue.

4 participants