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

Setup types for Robyn #192

Merged
merged 5 commits into from
May 8, 2022
Merged

Setup types for Robyn #192

merged 5 commits into from
May 8, 2022

Conversation

sansyrox
Copy link
Member

@sansyrox sansyrox commented May 2, 2022

Description

TODO:

  • To create generic types for rust files.
  • Fix docstrings

This PR fixes #130

@netlify
Copy link

netlify bot commented May 2, 2022

Deploy Preview for robyn canceled.

Name Link
🔨 Latest commit be532f6
🔍 Latest deploy log https://app.netlify.com/sites/robyn/deploys/6277c40da9e3960009ad6c60

@sansyrox sansyrox changed the title Initial Setup for types in Robyn [WIP] Initial Setup for types in Robyn May 2, 2022
robyn/processpool.pyi Outdated Show resolved Hide resolved
@sansyrox sansyrox changed the title [WIP] Initial Setup for types in Robyn Setup types for Robyn May 3, 2022
@sansyrox
Copy link
Member Author

sansyrox commented May 4, 2022

@Kludex @RobertCraigie , would it be possible for you folks to have a review at this PR?

Comment on lines 1 to 15
import os
import asyncio
import multiprocessing as mp
from inspect import signature
from robyn.events import Events
from .robyn import Server, SocketHeld
from .argument_parser import ArgumentParser
from .responses import jsonify, static_file
from .dev_event_handler import EventHandler
from .processpool import spawn_process
from .log_colors import Colors
from .ws import WS
from .router import MiddlewareRouter, Router, WebSocketRouter
from multiprocess import Process
from watchdog.observers import Observer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/PyCQA/isort would probably make your life easier here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this! How did I not know about this 🤣

robyn/__init__.pyi Outdated Show resolved Hide resolved
robyn/__init__.pyi Outdated Show resolved Hide resolved
class ArgumentParser(argparse.ArgumentParser):

def __init__(self) -> None:
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use ... instead of pass, and when you add a docstring, you don't need any. JFYK

Suggested change
pass
...

robyn/dev_event_handler.pyi Outdated Show resolved Hide resolved
robyn/dev_event_handler.pyi Outdated Show resolved Hide resolved
robyn/processpool.pyi Outdated Show resolved Hide resolved
robyn/processpool.pyi Outdated Show resolved Hide resolved
robyn/processpool.pyi Outdated Show resolved Hide resolved
robyn/__init__.pyi Outdated Show resolved Hide resolved
robyn/dev_event_handler.pyi Outdated Show resolved Hide resolved
@sansyrox
Copy link
Member Author

sansyrox commented May 7, 2022

@Kludex , can you please have a look again?

@sansyrox sansyrox merged commit 77064fb into main May 8, 2022
@sansyrox sansyrox deleted the type-stubs branch May 8, 2022 15:33
Copy link

@RobertCraigie RobertCraigie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sansyrox Sorry for the delay. It looks pretty good to me!


pass

def jsonify(input_dict: dict) -> str:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the type arguments for a generic type don't matter in that context you should still use Any or object in its place. This is because in strict mode, pyright will report an error if a type annotation is unknown (i.e. missing type arguments).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @RobertCraigie! I will include it in the next release! :D

show_files_listing: bool,
):
pass
def add_header(self, key: str, value: str):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the method just returns None it's generally preferable to be explicit about that to avoid any potential differences in inference between type checkers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! I will change it in the next release! :D

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.

Add Python stubs
3 participants