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 stubs for Flask-SocketIO #10735

Merged
merged 26 commits into from
Sep 25, 2023
Merged

Add stubs for Flask-SocketIO #10735

merged 26 commits into from
Sep 25, 2023

Conversation

sohang3112
Copy link
Contributor

No description provided.

@github-actions

This comment has been minimized.

@sohang3112
Copy link
Contributor Author

@miguelgrinberg I didn't specify the type of namespace because I'm not sure if the correct type is Namespace (from namespace module), str or if either of the two are possible.

@miguelgrinberg
Copy link

@sohang3112 namespace is always a str. The class-based namespaces are never passed as arguments there.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

3 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@sohang3112
Copy link
Contributor Author

@AlexWaygood I added flask_socketio.test_client.SocketIOTestClient.clients to tests/stubtest_allowlists/py3_common.txt since it's an undocumented attribute. But the CI test Third-party stubtest / Check third party stubs with stubtest (ubuntu-latest) (pull_request) is still raising an error:

error: flask_socketio.test_client.SocketIOTestClient.clients is not present in stub
Stub: in file /home/runner/work/typeshed/typeshed/stubs/Flask-SocketIO/flask_socketio/test_client.pyi
MISSING
Runtime:
{}

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 20, 2023

@AlexWaygood I added flask_socketio.test_client.SocketIOTestClient.clients to tests/stubtest_allowlists/py3_common.txt since it's an undocumented attribute. But the CI test Third-party stubtest / Check third party stubs with stubtest (ubuntu-latest) (pull_request) is still raising an error:

error: flask_socketio.test_client.SocketIOTestClient.clients is not present in stub
Stub: in file /home/runner/work/typeshed/typeshed/stubs/Flask-SocketIO/flask_socketio/test_client.pyi
MISSING
Runtime:
{}

tests/stubtest_allowlists/py3_common.txt is an allowlist for our standard-library stubs. You want to add the stubtest allowlist entry to a new stubs/Flask-SocketIO/@tests/stubtest_allowlist.txt file, not tests/stubtest_allowlists/py3_common.txt.

@github-actions

This comment has been minimized.

@sohang3112
Copy link
Contributor Author

@AlexWaygood You made some commits (eg. Fixed some obvious stuff) but I'm not sure how I can add these commits to my forked repo. Can you help with this?

@JelleZijlstra
Copy link
Member

@sohang3112 they are already in your fork. To get them locally, you should be able to simply run git pull.

@AlexWaygood AlexWaygood changed the title Added stubs for Flask-SocketIO Add stubs for Flask-SocketIO Sep 22, 2023
@sohang3112
Copy link
Contributor Author

sohang3112 commented Sep 23, 2023

I included type stub for SocketIOTestClient.get_received, but I'm getting this confusing error in the test Third-party stubtest / Check third party stubs with stubtest (ubuntu-latest) (pull_request):

error: flask_socketio.test_client.SocketIOTestClient.get_received is not present in stub
Stub: in file /home/runner/work/typeshed/typeshed/stubs/Flask-SocketIO/flask_socketio/test_client.pyi
MISSING
Runtime: in file /tmp/tmpjx8or4co/lib/python3.10/site-packages/flask_socketio/test_client.py:220
<function SocketIOTestClient.get_received at 0x7f579dd8ea70>

error: flask_socketio.test_client.SocketIOTestClient.get_recieved is not present at runtime
Stub: in file /home/runner/work/typeshed/typeshed/stubs/Flask-SocketIO/flask_socketio/test_client.pyi:34
def (self: flask_socketio.test_client.SocketIOTestClient, namespace: Union[builtins.str, None] =) -> builtins.list[TypedDict('flask_socketio.test_client._Packet', {'name'?: builtins.str, 'args'?: Any, 'namespace'?: builtins.str})]
Runtime:
MISSING

First it says get_recieved is present in the library but missing in the type stub. Then it says it's present in the type stub but missing in the library??!
@miguelgrinberg @AlexWaygood do you have any idea why this could be happening??

@Akuli
Copy link
Collaborator

Akuli commented Sep 23, 2023

You have a typo: get_received is not same as get_recieved.

@sohang3112
Copy link
Contributor Author

You have a typo: get_received is not same as get_recieved.

Thanks! I'll fix that.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@sohang3112
Copy link
Contributor Author

@AlexWaygood As you can see, all the CI tests are passing now. Please let me know if any other change is required before merging this pull request.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
Copy link
Member

This looks pretty good to me now. @miguelgrinberg, did you have any other comments? (No worries if you don't have time to take another look; you're certainly not obliged to!)

@miguelgrinberg
Copy link

@AlexWaygood No more comments from my side, looks good. :)

@AlexWaygood AlexWaygood merged commit da187a9 into python:main Sep 25, 2023
47 checks passed
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

5 participants