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

missing type stubs for libcst.native #862

Closed
jakkdl opened this issue Feb 17, 2023 · 6 comments
Closed

missing type stubs for libcst.native #862

jakkdl opened this issue Feb 17, 2023 · 6 comments
Labels
wontfix This will not be worked on

Comments

@jakkdl
Copy link
Contributor

jakkdl commented Feb 17, 2023

E.g. libcst.parse_module is wonderfully typed, but libcst.native.parse_module has no stubs.
I don't think it should be too bad to write .pyi files for the functions in libcst.native despite them being hooked in with PyO3?

@zsol
Copy link
Member

zsol commented Feb 17, 2023

You're not supposed to call libcst.native.parse_module, that's considered a private API (maybe this should be documented better). libcst.parse_module is the API users are expected to call, and that will delegate to libcst.native.parse_module if LIBCST_PARSER_TYPE=native env var is set.

@jakkdl
Copy link
Contributor Author

jakkdl commented Feb 18, 2023

The normal parser crashes on our test files in flake8-trio, and presumably any time a user wants to lint code containing new grammar, so I see no reason not to always call the native parser? I suppose I could set the env var in the program before calling parse_module, though that feels kinda cumbersome.

@zsol
Copy link
Member

zsol commented Feb 18, 2023

I suppose I could set the env var in the program before calling parse_module, though that feels kinda cumbersome.

Agreed, libcst should call the native parser by default. I was hesitant to make that change, because there are some features of parse_module that the native parser doesn't support (most importantly limiting the [python_version](https://github.com/Instagram/LibCST/blob/main/libcst/_parser/types/config.py#L80), but I don't think anyone will have the time to work on these features anytime soon. At some point the burden of setting that env var will outweigh the benefits of these missing features...

@jakkdl
Copy link
Contributor Author

jakkdl commented Feb 18, 2023

ah that's a shame, but I'll just set os.environ for now then.

Better documentation does sound good, and/or adding some leading underscores / not exporting parser.native.

@Zac-HD
Copy link
Contributor

Zac-HD commented Feb 20, 2023

Yeah, I went with https://github.com/Zac-HD/shed/blob/f3653b21987172d4c7b3b369d8067116f5edc919/src/shed/_codemods.py#L39-L48 - it's a bit awkward but not too bad and libcst is still so much nicer overall🙂

@zsol
Copy link
Member

zsol commented May 25, 2023

I'm going to close this out now that #929 has been merged.

@zsol zsol closed this as completed May 25, 2023
@zsol zsol added the wontfix This will not be worked on label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants