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

cython compatibility #674

Closed
munechika-koyo opened this issue Dec 3, 2020 · 15 comments
Closed

cython compatibility #674

munechika-koyo opened this issue Dec 3, 2020 · 15 comments

Comments

@munechika-koyo
Copy link

I work on python-based programming using a certain class implemented by cython.
Although the conventional language server (e.g. Microsoft python language server) can show the docstrings written in a cython-defined class, pylance doesn't handle them.
I really appreciate it if developers take it into account.
Thank you in advance.

@github-actions github-actions bot added the triage label Dec 3, 2020
@judej
Copy link
Contributor

judej commented Dec 3, 2020

@munechika-koyo, thanks for the report. Which class/lib are you using? and could you give us a a code snippet that demonstrates the problem please?
thanks

@judej judej added the waiting for user response Requires more information from user label Dec 3, 2020
@github-actions github-actions bot removed the triage label Dec 3, 2020
@munechika-koyo
Copy link
Author

I use the Raysect library which is the ray-tracing tool, especially for the plasma analysis.
This library is mostly written in cython.

In the case of using Microsoft as a python language server, the docstring popup looks like below:
image

In contrast to the above, in case the of using Pylance:
スクリーンショット 2020-12-04 165624

Both of them are shown when the cursor hovers on the World class implemented in Raysect.

Here, I put some environmental information.

Environment data

  • VS Code version: 1.51.1
  • Pylance extension version: v2020.12.0
  • OS and version: Windows 10, and I use Remote-WSL.
  • Python version: 3.6.10 64-bit in Anaconda env.

@jakebailey jakebailey added compiled and removed waiting for user response Requires more information from user labels Dec 4, 2020
@jakebailey
Copy link
Member

Thanks for the info. Yes, this appears to be compiled via cython. MPLS scraped every compiled module, Pylance does not.

@cnaccio
Copy link

cnaccio commented Dec 24, 2020

Just noticed this issue as well using the Zipline library which uses cython throughout the project. Switched to the Microsoft language server for now, but def prefer Pylance.

@polm
Copy link

polm commented Jun 15, 2021

Just a note that this is the same as #166, which is the oldest / most active topic on the issue of Cython support.

EDIT: Ah, actually #166 is a bit more general than Cython, though still on the same topic.

@judej
Copy link
Contributor

judej commented Apr 20, 2022

Closing this as we have no plans to scrape compiled modules in Pylance

@judej judej closed this as completed Apr 20, 2022
@alexreg
Copy link

alexreg commented Apr 20, 2022

@judej It's understandable that you may not wish to tackle this issue immediately (due to limited "bandwidth") — but it's also very disappointing, considering how widespread Cython is. Wouldn't it be better simply to triage this?

Also, I don't think you necessarily need to "scrape compiled modules" to offer Cython support.

@erictraut
Copy link
Contributor

erictraut commented Apr 20, 2022

I think the right answer here is for Cython toolchain to add support for generating stub files with docstrings. Those stubs can then be distributed along with the library. Any other approach I can think of (including scraping based on runtime reflection) will produce results that leave significant gaps in terms of completeness and correctness. The maintainers of Cython are in a much better place to do this work than the Pylance team. We are happy to work with the Cython community in an advisory role.

@alexreg
Copy link

alexreg commented Apr 20, 2022

@erictraut Agreed, that approach makes a lot of sense. I'll create a new issue on the Cython repo, to make the developers there aware of things.

@scoder
Copy link

scoder commented Apr 21, 2022

I think the right answer here is for Cython toolchain to add support for generating stub files with docstrings. Those stubs can then be distributed along with the library. Any other approach I can think of (including scraping based on runtime reflection) will produce results that leave significant gaps in terms of completeness and correctness.

@erictraut, could you elaborate on this? Bothering users with having to generate and distribute additional stub files seems really annoying for everyone. What type information do you get from source code and stubs that is not available from the normal Python runtime introspection? Isn't runtime information also cheaper and more universal to collect than stub files that a) have to be made available by someone, b) have to be available where they are used and c) have to be parsed as source code?

@erictraut
Copy link
Contributor

erictraut commented Apr 21, 2022

Yes, users who consume a library should not need to discover, download, or generate stub files. Type information should always be included in a library package and downloaded and installed as part of the library package. For libraries that are generated entirely or partly using the Cython compiler, Cython could automatically generate these stubs. A library author could then include the stubs alongside the compiled binaries within their package.

Runtime information obtained through introspection of a compiled module is insufficient for a number of reasons. First, accessing runtime type information requires loading and running the code in the library. As a rule, static tools like Pylance never load or run code. Doing so would be a major security problem. All type information used by Pylance must be in a declarative form — type annotations that are inlined in Python source (".py") files or included in stub (".pyi") files. Second, runtime type information is incomplete. It doesn't include function parameter or return types. These types are known by the Cython compiler, so it should be able to emit stubs that include this type information.

Here's documentation we've created for library authors that discusses the type information that should be included in a library if they would like it to work well with Pylance and other static analysis tools (language servers, type checkers, linters, etc.).

@scoder
Copy link

scoder commented Apr 21, 2022

Runtime information obtained through introspection of a compiled module is insufficient for a number of reasons. First, accessing runtime type information requires loading and running the code in the library. As a rule, static tools like Pylance never load or run code.

I understand that argument. It's also a dedicated goal of PEP-484 & friends.

Doing so would be a major security problem.

Assuming that we are talking about IDEs, AFAIR Python's setuptools has autoimport features, so any Python package that I install in my venv can execute code when I launch my Python runtime. Or can overwrite other packages with its own code that I would commonly import. Whether a static analysis tool can also trigger code doesn't seem to add anything substantial to that threat.

Regardless, this is the wrong place to discuss something like this (yet again). I'd just like to see people stop claiming that importing a module is a security threat, while installing software from untrusted sources is the actual threat.

Second, runtime type information is incomplete. It doesn't include function parameter or return types.

That's what __annotations__ is for. These dicts provide strings in Cython that can be parsed as type information.

And it looks like users have exploited this before to generate stub files from Python annotations: python/mypy#7542
That approach (stubgen) seems generally helpful.

I think the main issue is that special methods in CPython cannot expose their signature. And Cython doesn't (currently?) have a way to work around that. But the only real issues here are with __init__, __new__ and __call__, since all other special methods have a fixed signature. So, the missing part of the type information is overall very limited.

These types are known by the Cython compiler, so it should be able to emit stubs that include this type information.

I agree, see cython/cython#3150
Regardless, since stubgen exists, it would probably get users pretty far already to use it.

@JMPZ11
Copy link

JMPZ11 commented Jun 22, 2022

Forgive the french, but closing this is horse shit. Python's c(++) interop is a selling point, and pylance not supporting Cylance is a HUGE miss; especially considering we know it can be done ala MPLS.

Fix this please.
Thanks

@FeldrinH
Copy link

@erictraut Agreed, that approach makes a lot of sense. I'll create a new issue on the Cython repo, to make the developers there aware of things.

@alexreg Did you create that issue? I looked at the Cython repo issue tracker and couldn't find a relevant issue, unless you count cython/cython#3150, but the original request there seems to be about a different thing.

@scoder
Copy link

scoder commented Dec 16, 2023 via email

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

No branches or pull requests

10 participants