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

Type annotate the compiler #2030

Open
allison-casey opened this issue Mar 31, 2021 · 11 comments
Open

Type annotate the compiler #2030

allison-casey opened this issue Mar 31, 2021 · 11 comments

Comments

@allison-casey
Copy link
Contributor

In going through and cleaning up the compiler I'm noticing a number of issues associated with the fact that nothing is annotated. Lots of hand waving around expected types that make the compiler a lot harder to read and debug (potentially unbound varaibles, a compiler crash because ast.parse can't take None as a filename even though HyASTCompiler can be instantiated without one, etc). I'd suggest type annotating at least the compiler to make future development easier and less bug prone. I have a branch with the makings of a fully annotated compiler module that I can flesh out and pr if there's interest. Type annotations are only getting better and faster with each python release, including in the core ast module, and it would be a nice train to jump on.

@Kodiologist
Copy link
Member

I haven't really stayed in the loop with respect to type annotations, and have gotten the impression that Python supports adding them but does nothing with them itself. How would we use them?

@allison-casey
Copy link
Contributor Author

allison-casey commented Mar 31, 2021

python does nothing with them, but whatever language server protocol your editor uses can use them to do static analysis and find potential bugs (either through MyPy or Pyright) so it's purely a development bonus to get better introspection and editor error messages when mucking with the code base. So it would be a non breaking drop in refactor for what we already have.

@Kodiologist
Copy link
Member

Shouldn't they be validated somehow? Suppose somebody who isn't using a language server (e.g., me) submits a PR that's incorrect with respect to the type annotations.

@allison-casey
Copy link
Contributor Author

it can be added to travis and error out if someone introduces a pr that isn't typed properly

@Kodiologist
Copy link
Member

Okay, sounds reasonable, then.

@allison-casey allison-casey self-assigned this Apr 2, 2021
@allison-casey allison-casey removed their assignment Jun 8, 2021
@lafrenierejm
Copy link
Contributor

I'd be glad to take a stab at this.

I noticed that some of the Google-style docstrings already contain type annotations. Am I correct in assuming the preference would be to use native, in-line type hints overall docstring conventions and separate .pyi files?

@lafrenierejm
Copy link
Contributor

Is there a preference for which type checker is adopted? Mypy is the de facto standard in the Python ecosystem, but there are alternatives with slightly different trade-offs. Most notable alternatives that I'm aware of:

@Kodiologist
Copy link
Member

Kodiologist commented Jan 23, 2022

I can't speak to those questions, but beware that it appears that getting Hy to pass a typecheck is a massive job, even if you only add a little typing information (see #2130, which probably should've been linked to this issue earlier). The important thing is that we have the type hints checked as a GitHub Action, so we can be sure they're correct and stay correct.

@Kodiologist
Copy link
Member

So, the smoothest route is likely to begin by getting the codebase to pass a typecheck as it stands, with few type hints, and then add the desired new hints.

@lafrenierejm
Copy link
Contributor

Thanks for pointing me to #2130. Based on the conversation there it seems like there's at least a slight preference for pyright as the type checker, so I've started working against it. I'll raise a WIP PR once I have something worth looking at.

@Kodiologist
Copy link
Member

#2235 was an attempt to fulfill this issue, and should probably be looked at by anybody who's thinking of taking this on.

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

3 participants