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

Add types + Make library PEP-561 compatible #528

Open
Andrioden opened this issue Sep 3, 2023 · 5 comments
Open

Add types + Make library PEP-561 compatible #528

Andrioden opened this issue Sep 3, 2023 · 5 comments

Comments

@Andrioden
Copy link

Andrioden commented Sep 3, 2023

Why

I love types, and want everything in python typed as strongly as possible within reason. This creates more readable code and makes refactoring easier and safer. If i debug my code that depend on ably code, i find myself stepping into untyped ably code, this makes it harder to read.

The following import triggers an mypy error

game\utils\event_publisher.py

from ably import AblyRest

Running mypy .

game\utils\event_publisher.py:6: error: Skipping analyzing "ably": module is installed, but missing library stubs or py.typed marker  [import]
game\utils\event_publisher.py:6: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

In addition i see the very prominent Channel.publish is not typed.

My suggestions are

  • Make your library PEP-561 compatible
  • Type all public exposed code
  • Add mypy or other type checking to your CI/CD pipelines
  • If possible type the whole repo

Read more here

┆Issue is synchronized with this Jira Task by Unito

@sync-by-unito
Copy link

sync-by-unito bot commented Sep 3, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3842

@Andrioden
Copy link
Author

The "fix" in my repo is to add the following to my config

pyproject.toml

[[tool.mypy.overrides]]
module = "ably.*"
ignore_missing_imports = true

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 3, 2023

@Andrioden even I agree with you. Typing surely brings more power to the code. In the subsequent iterations, we will surely cover missing types from the code.

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 3, 2023

Feel free to raise more suggestions and create a PR if possible : )

@Andrioden Andrioden changed the title Make library type/PEP-561 compatible Add types + Make library PEP-561 compatible Sep 3, 2023
@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 4, 2023

Related to #480

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants