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

Fix type annotations in the str | bytes #945

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

1st
Copy link
Contributor

@1st 1st commented Aug 27, 2023

I've faced an issue with Mypy that it complains about passing string to the ws.send() method. And found that you use bytes or str instead of the Python built-in pipe to be str | bytes.

Checked with Mypy and it works fine now.

@1st 1st changed the title Fix type annotations in the with str | bytes Fix type annotations in the str | bytes Aug 27, 2023
@engn33r engn33r merged commit d19c3d1 into websocket-client:master Aug 28, 2023
@engn33r
Copy link
Collaborator

engn33r commented Aug 28, 2023

Thanks for the PR. mypy will be added to CI after more mypy warnings are resolved.

@1st
Copy link
Contributor Author

1st commented Aug 28, 2023

Thanks for the PR. mypy will be added to CI after more mypy warnings are resolved.

Thanks. If will face any other issues will make a follow-up fix

engn33r added a commit that referenced this pull request Sep 1, 2023
@engn33r
Copy link
Collaborator

engn33r commented Sep 1, 2023

I reverted this PR with a3f4c3a because it is breaking Python 3.8 and 3.9 CI and compatibility. The | union operator was added in Python 3.10 in PEP 604.

@1st
Copy link
Contributor Author

1st commented Sep 1, 2023

Yes. It's modern syntax. If you need an old syntax then create a branch for supporting old version of Python. Or you want to have a single version version that covers old and new versions?

@engn33r
Copy link
Collaborator

engn33r commented Sep 2, 2023

There is only a single version of the library to support all versions of python that are not EOL. The supported versions right now are 3.8+, listed in setup.py

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 this pull request may close these issues.

None yet

2 participants