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

experiment with shipping type annotations #2638

Open
mmerickel opened this issue Jun 6, 2016 · 29 comments
Open

experiment with shipping type annotations #2638

mmerickel opened this issue Jun 6, 2016 · 29 comments

Comments

@mmerickel
Copy link
Member

mmerickel commented Jun 6, 2016

It'd be cool if pyramid could ship type annotations for its public APIs. I talked to the pycharm folks at their booth last week and there are apparently 3 ways to do this given that we want to keep working on py2 and py3:

  1. Use the comment-based type annotation syntax in pyramid source which is compatible with py2.
  2. Use separate stub files that are shipped to the typeshed repo. https://github.com/python/typeshed/
  3. Use separate stub files distributed in the pyramid package.

It would be helpful to evaluate the pros and cons of the various approaches with the primary goal being to minimize the maintenance effort required. For example, a build hook that could check that type hints were defined for public APIs.

On a side note, I think the commant syntax is ugly and that it should be considered the worst of the 3 options.

Related #2387

NOTE: This issue is vague because I only sort of know what I'm talking about.

@stevepiercy
Copy link
Member

Option 1 and 3 require a release of Pyramid (con), whereas 2 does not (pro). That alone makes option 2 the most appealing to me.

@mmerickel
Copy link
Member Author

Option 1 and 3 require a release of Pyramid (con), whereas 2 does not (pro). That alone makes option 2 the most appealing to me.

These are type annotations, not documentation. They are inherently coupled to the source of the release. I think this is not a concern.

@stevepiercy
Copy link
Member

Are you saying that there shall be no bugs or other mistakes in the type annotations when added to the Pyramid package, or that if there are then it is no big thing?

@mmerickel
Copy link
Member Author

mmerickel commented Jun 6, 2016

I think if there are bugs with them then they could simply get fixed in a later release. Again that's just my first impression as I have not used type annotations myself.

@tseaver
Copy link
Member

tseaver commented Jun 6, 2016

-1 to option 2: annotations have to stay tightly in-sync with code: in fact, we should find a way to "test" them so that we don't see them drift. I see no benefit in creating an out-of-tree "authority" for them which can drift (and will, inevitably) from changes as Pyramid releases are made.

@iivmok
Copy link

iivmok commented Nov 2, 2016

I would love to see Pyramid type annotations, personally I like the 3rd option the most. I use PyCharm and type annotations a lot, and marking request as WebOb Request class helps a whole lot, especially when I first started using Pyramid.

@ztane
Copy link
Contributor

ztane commented Nov 2, 2016

I would like to see the option 4: stop supporting Python 2 and use real annotations.

Though a bigger problem is that nothing supports interfaces.

@mmerickel
Copy link
Member Author

Though a bigger problem is that nothing supports interfaces.

Yeah zope.interface is a big issue for us in Pyramid. It looks like python/mypy#1240 is slowly working on this.

@domenkozar
Copy link
Member

Most of pyramid "exposed" api isn't zope.interface and thus would benefit from having types :)

@mmerickel
Copy link
Member Author

I'm not sure that's true. We have two major problems in terms of typing / linting.

  1. Almost all of our APIs are defined in terms of zope interfaces such as a view signature which accepts (context: Any, request: pyramid.interfaces.IRequest) or config.set_authentication_policy(policy: pyramid.interfaces.IAuthenticationPolicy).

  2. Some of Pyramid's public api is configurable at runtime as well which is difficult to type at all (speaking as someone with minimal mypy experience). For example config.add_directive adds extra methods to the configurator and config.add_request_method adds extra methods to the request.

@domenkozar
Copy link
Member

domenkozar commented Jun 22, 2018

  1. Yes, those could be defined as Any until zope.interface plugin exists.

  2. Could define http://mypy.readthedocs.io/en/latest/protocols.html for default Configurator, then the rest of custom methods typing is up to the user.

(close button is too close)

@pior
Copy link

pior commented Aug 11, 2018

Is there a plan to work on typing already?

I don't see any mention of it in the 2.0 feature list #2362.

I would love to help with stubs/comments for 1.x since I'm using it at work, but it would also be interesting to plan for Py3.

@ztane
Copy link
Contributor

ztane commented Aug 13, 2018

Please when talking with pycharm folks, tell them we want to type with zope interfaces too :D

@mmerickel
Copy link
Member Author

mmerickel commented Aug 13, 2018

Here is, in my eyes, what needs to happen to get typing into Pyramid. The goal would be to support typing via annotations and not via stub files or comment syntax. This is open for debate but this is the roadmap I have in my head.

  • refactor the repo structure first refactor repo structure #3022 (this should be coming in the upcoming 1.10 release)
  • drop support for Python 2 Drop support for Python 2 #2903 (this should be happening after the 1.10 release)
  • experiment / come up with a strategy to handle the configurator and z.i issues (using Any or something else) and test it out on pycharm and mypy (no one has really taken this on yet that I know of)
  • actual PR to pyramid to add typings including mypy steps to our tox -e lint config

If I were someone interested in doing this and starting from scratch I would probably look at what websauna is doing. It is a framework built on top of pyramid and is py3-only with annotation-based typing.

@jtrakk
Copy link

jtrakk commented Dec 30, 2019

Yes, those could be defined as Any until zope.interface plugin exists.

Here it is. https://github.com/Shoobx/mypy-zope

@duynhaaa
Copy link
Contributor

duynhaaa commented Dec 4, 2024

Hi, has anyone started working on this? If not, I'm willing to contribute, but definitely need some guidance from the maintainers. I really love Pyramid's design and architect and hate seeing it slowly becoming a relic of the past.

P/s: I'm starting most of my personal projects with Pyramid, so the investment has already been made.

@mmerickel
Copy link
Member Author

I'm happy to review as best as I can - I just have limited familiarity with Python's type annotation system and haven't used things like mypy yet.

@luhn
Copy link
Contributor

luhn commented Dec 4, 2024

imo the best place to start would be with webob, since pyramid.request.Request is the object that's most interacted with it and a large chunk of it is inherited from webob.Request.

@luhn
Copy link
Contributor

luhn commented Dec 4, 2024

For Pyramid itself, the biggest outstanding question to me is how to handle Configurator, especially add_directive and add_request_method. Those are making runtime modifications to the Configurator and Request, respectively, and I'm not sure how to get that information to mypy.

@mmerickel
Copy link
Member Author

mmerickel commented Dec 4, 2024

You're correct that add_directive and add_request_method are the true challenges of typing pyramid. My general impression of how these would need to be handled is with external type extesions. Webob and Pyramid would maintain their specific types for all builtin behaviors on pyramid.config.Configurator and webob.request.Request and pyramid.request.Request. User code would then default to using these types. However we would need a way for a package to provide a type extension alongside the directive and the user code would need a way to use those types within their own code for type checking. Maybe there would be a way to then automate pulling those types together for mypy, but it starts with the basics:

  • You define a directive, you provide a type extension to pyramid.config.Configurator.
  • You define a request method, you provide a type extension to pyramid.request.Request.

So you define extension a:

# extension a

class ConfiguratorWithExtensionA(Configurator):
    def extension_a(...) -> None:
        pass

def includeme(config):
    config.add_directive(..., 'extension_a')

Later a user could do something like:

from pyramid.config import Configurator
from extension_a import ConfiguratorWithExtensionA

class MyConfigurator(Configurator, ConfiguratorWithExtensionA):
    pass

config: MyConfigurator = Configurator()
with config:
    config.include('extension_a')
    config.extension_a(...)  # <-- goal is that we can type-check this invocation correctly

app = config.make_wsgi_app()

Again I'm showing how naive I am with Python types but I think the general strategy makes sense from what I've seen.

@merwok
Copy link
Contributor

merwok commented Dec 5, 2024

I think this may not work, as the analysis would find that Configurator() does not return a MyConfigurator instance, but there is typing.cast to override analysis.

@duynhaaa
Copy link
Contributor

duynhaaa commented Dec 5, 2024

I think at the moment, the type checking of such dynamic objects (whose interfaces can be modified imperatively after the initial declaration, like the pyramid.config.Configurator with .add_directive as mentioned) to the users because:

  1. Static type checking only works best on information that has already been statically and declaratively encoded in the source code (e.g., declarations of classes, functions and variables). The only dynamic thing that the type checking system can support is generic type, which requires all the possible derived types be declared, either explicitly or implicitly (if you squint your eyes enough). Think of it like conditional branching but for types.

  2. Other languages with statically typed nature, such as Go and C/C++, also have to enter "type-unsafe" zone at some point because it doesn’t make sense or worth the effort to perform "type-gymnastic" just to make the only 5% unsafe code safe when tests are sufficient enough. Such cases usually happen when either the data is unstructured or the nature of the problem is too hard to define as a whole.

So in conclusion, unless we know all the possible future combination, which basically is omniscience at this point, or willing to introduce a complicated type system, which usually is not a good idea (mandatory example: https://github.com/gruhn/typescript-sudoku), there’s no need to have the framework fully typed to a tee.

TL;DR: Add typings for things we know and leave the unknown to the users.

@mmerickel
Copy link
Member Author

Yes absolutely the initial goal is just to get the core framework to have better types. The extensibility of add_directive and add_request_method can come later.

A lot of core types in the framework are already defined as zope interfaces in pyramid.interfaces so ideally we’d have a way to reuse that but I don’t think z.i is expressive enough to really do what you want with a protocol in mypy.

@duynhaaa
Copy link
Contributor

duynhaaa commented Dec 5, 2024

So, should I start with webob like @luhn suggested or go straight to pyramid?

I will try to get something working soon (expecting by the end of next week).

@mmerickel
Copy link
Member Author

I’d probably suggest starting with pyramid simple because webob’s codebase is in a weird cleanup phase that needs some love so I think it’d introduce annoying stop energy to start there.

@duynhaaa
Copy link
Contributor

duynhaaa commented Dec 5, 2024

@mmerickel Is webob in the middle of a rewrite or something?

Either way, I'll start working on pyramid.

@mmerickel
Copy link
Member Author

No not a rewrite. I looked at it again and it's not as bad as I thought, things are in a pretty decent state atm. You could definitely add types to webob. MultiDict will be tricky but hey :-)

Ultimately it'd be helpful because you'll want to inherit those types into pyramid.request.Request and pyramid.response.Response.

@mmerickel
Copy link
Member Author

mmerickel commented Dec 5, 2024

I think this may not work, as the analysis would find that Configurator() does not return a MyConfigurator instance, but there is typing.cast to override analysis.

Good point yeah I suppose a cast would be required. It feels like there's not going to be a way around that but most of my typing experience is in static typing domains (or in the overly flexible z.i per-object typing domain which won't work with python's annotations afaik). Python's annotations with mypy are in a middle ground on the spectrum that I don't fully understand but is closer to static I suppose.

@merwok
Copy link
Contributor

merwok commented Dec 5, 2024

IIUC Python’s typing is static, but smart enough to support duck typing (with Protocols).
But the issue of adding methods is another thing.

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