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 type annotations #530

Closed
graingert opened this issue Mar 8, 2017 · 28 comments
Closed

mypy type annotations #530

graingert opened this issue Mar 8, 2017 · 28 comments

Comments

@graingert
Copy link
Contributor

As this is a Python >= 3.5 library, it would be easy for callers of this library to support or ignore type annotations if they were added.

@subyraman
Copy link
Contributor

Would you be interested in helping to add type annotations?

@graingert
Copy link
Contributor Author

Yup, can get you setup with a simple mypy config

@alexyer
Copy link

alexyer commented Mar 28, 2017

Is there any progress? I can help with it.

@jgelens
Copy link

jgelens commented Nov 29, 2017

I'm slowly adding some annotations for usage in my own project . Do you want to integrate the annotations in the Sanic code itself or leave it as a .pyi stub file (that defines the types in a separate file)

@feluxe
Copy link

feluxe commented Nov 18, 2018

Cython now understands PEP484 and PEP526 type annotations.

That means you can run the Cython compiler on (pure) Python files and get decent speed improvements.

On my machine it takes 9.79 seconds to run func_a(10,10,10) 5000000 times:

def f(x):
    return x ** 2 - x

def func_a(a, b, N):

    s = 0
    dx = (b - a) / N

    for i in range(N):
        s += f(a + i * dx)

    return s * dx

The same code with some type annotations runs in 2.40 seconds (compiled with cython).

import cython as c


def fd(x:c.double):
    return x ** 2 - x


def func_d(a:c.double, b:c.double, N:c.int):

    i:c.int

    s = 0
    dx = (b - a) / N

    for i in range(N):
        s += fd(a + i * dx)

    return s * dx

Maybe this is something sanic could make use of?

@ahopkins
Copy link
Member

There are two separate things going on here. First, it probably makes sense to create some,stub files to help developers.

Second, making use of Cython. We've talked about this before and it may be in the table for some of the features in 2019. But I would suggest moving this conversation to its own thread. And since it is not a specific enhancement, I'd suggest starting a conversation on the community forums.

@ahopkins
Copy link
Member

Anyone interested to start working in some stub files? @sjsadowski and @yunstanford, 19.03?

@yunstanford
Copy link
Member

Yeah, I'd like to optimize it with Cython. I've done some work before.

@vltr
Copy link
Member

vltr commented Nov 19, 2018

Yeah, this might give some performance boost but I think we should be able to have a pure Python implementation as a fallback in case installing binaries is not possible (ex: using pypy (or any other Python implementation that's not CPython); container Linux images without the proper build tools (there's an issue describing this problem here) and perhaps some other scenarios).

@feluxe
Copy link

feluxe commented Nov 20, 2018

@vltr The compiled version could be uploaded to pypi as csanic.

@vltr
Copy link
Member

vltr commented Nov 20, 2018

@feluxe yeah, well, I not much convinced in using the letter c as prefix since it reminds C. I know that Cython generates C code (and has to be compiled as such), but it is far away from being as performant as a C code made for that same specific purpose (with Python bindings - in pure C or Cython). I know there are cases where C code and Cython code can "match" each other in performance, but in general they don't. Sometimes you can write a Python code that's faster than C - if talking about micro-optimizations. Anyway, this is a long discussion I guess 😅

@stale
Copy link

stale bot commented May 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@ahopkins
Copy link
Member

See also #1318. We are talking about using cython for some other performance enhancements (specifically in the router). It might be nice to also see if we can use it to clean up some of the performance issues noted here: https://community.sanicframework.org/t/making-sanic-even-faster/298

@abatilo
Copy link

abatilo commented Jun 9, 2019

Is anyone still working on this? If possible, I'd like permission to add some auto generated stubs to the official typeshed repo, and then to start working in actual type information gradually over time.

@ahopkins
Copy link
Member

You definitely do not need permission ... Nonetheless: permission granted

@abatilo
Copy link

abatilo commented Jun 10, 2019

It's part of the typeshed rules to get explicit permission:
https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#adding-a-new-library

:P

Thank you though!

@viniciusd
Copy link
Contributor

Are we good with moving forwards type hinting on sanic?

@ahopkins
Copy link
Member

We have started adding it in certain places. Hopefully we will see more of it.

@viniciusd
Copy link
Contributor

Awesome, I am willing to assist on this

@ahopkins
Copy link
Member

@viniciusd hired! 💪

I'd suggest to start a PR and mark it "WIP". Let me know if you have any questions, or need any direction.

@viniciusd
Copy link
Contributor

Great! So I am gonna start type hinting sanic. I should take a better look at the source code so I can make that incremental, probably performing the work on a [sub]module basis.

Thoughts on taking Sanic up to 3.6 though? PEP-526 improves the type hinting syntax

@ahopkins
Copy link
Member

Starting with v19.6 we did bump to 3.6

@LiraNuna
Copy link
Contributor

What's the status of this? I would love to assist if needed

@viniciusd
Copy link
Contributor

viniciusd commented Sep 20, 2019

It would be great, @LiraNuna!

Now I am free enough to contribute. I feel like creating a tracking issue so it is all organized and neat at one place, idk.

Anyway, I guess we can start by adding mypy to the project and fixing the initial issues?

I propose:

  1. Adding mypy to the project
  2. Fixing whatever issues pop-up
  3. Start adding type hint to Sanic's main interface, e.g., request.py and response.py

I will start by taking a look at how to set-up the dev environment and checking Sanic's travis-ci pipelines.

@viniciusd
Copy link
Contributor

viniciusd commented Sep 20, 2019

Just created a WIP PR at #1682 in order to address (1).

However, it should also address (2) in the very near future. Otherwise, its build won't succeed

@viniciusd
Copy link
Contributor

Not WIP anymore, #1682 adds a type checking pipeline

@graingert
Copy link
Contributor Author

Amazing thanks so much!

@ahopkins
Copy link
Member

For posterity's sake, #2006 added a lot of type annotations.

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

No branches or pull requests