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

compas imports - work with pylance intellisense #621

Closed
yck011522 opened this issue Sep 23, 2020 · 13 comments
Closed

compas imports - work with pylance intellisense #621

yck011522 opened this issue Sep 23, 2020 · 13 comments

Comments

@yck011522
Copy link
Contributor

Feature Request

Pylance is a feature rich python language plugin for VSCode. It provide code analysis and intellisense.

The intellisense function works with python default functions and types and almost all other libraries I used (such as numpy). However it does not work with compas.

I suspect this is caused by the 2-layer-deep import routing that compas uses. If I write the full path, or in fact keep the default full path that Pylance automatically creates, the intellisense works, See below where intersection_line_plane is yellow, that one works.

2020-09-23 23_22_21

I think @Achillx has faced similar problem.

@yck011522 yck011522 changed the title compas not working with pylance intellisense compas imports - work with pylance intellisense Sep 23, 2020
@Achillx
Copy link
Contributor

Achillx commented Sep 23, 2020

I've posted an issue on Pylance's Github about it. You can check it here, but that is the summary:
If you want your package to work correctly with static type checkers, you should either avoid using wildcard imports or replace dynamic all expressions with static expressions like all = ['cat', 'dog'].

@yck011522
Copy link
Contributor Author

Arh, that's an interesting reply from them. Perhaps it is modifiable from compas's side.

Pylance is based on Pyright, which is a static type analyzer. It is able to evaluate expressions like __all__ = ['cow'], but it cannot evaluate something as dynamic as __all__ = [name for name in dir() if not name.startswith('_')].

This of course will increase the maintenance work on compas development. I personally like the goal of this two-level import in compas where I believe is to shield users from internal implementation changes from version to version, I just don't know how much more extra maintenance work there is to be done.

@tomvanmele
Copy link
Member

tomvanmele commented Sep 26, 2020

do you know if it can work with *.pyi files to detect the contents of packages and modules?

@yck011522
Copy link
Contributor Author

I do not know.

@gonzalocasas
Copy link
Member

do you know if it can work with *.pyi files to detect the contents of packages and modules?

I think that would work. We would need to auto-generate them otherwise the effort to keep them up-to-date would be too high.

@tomvanmele
Copy link
Member

what seems to yield the correct result is to add a __init__.pyi at every second level package. so for example compas.datastructures.__init__.pyi. this is more or less what numpy does (all of there __all__ variables are also dynamically generated), albeit at the import levels that make sense for them.

Pylance correctly autocompletes if i add this to compas.datastructures.__init__.pyi

from .mesh import BaseMesh
from typing import Any

class Mesh(BaseMesh):
    bounding_box: Any = ...
    bounding_box_xy: Any = ...
    collapse_edge: Any = ...
    connected_components: Any = ...
    cut: Any = ...
    dual: Any = ...
    face_adjacency: Any = ...
    flip_cycles: Any = ...
    is_connected: Any = ...
    smooth_centroid: Any = ...
    smooth_area: Any = ...
    split_edge: Any = ...
    split_face: Any = ...
    transform: Any = ...
    transformed: Any = ...
    unify_cycles: Any = ...
    quads_to_triangles: Any = ...
    def transform_numpy(self, M: Any) -> None: ...
    def to_trimesh(self) -> None: ...
    def to_quadmesh(self) -> None: ...

however, it seems a bit complicated to automate. stubgen generates stubs per package / module and not at the level they have been dynamically pulled up to.

perhaps one could automate collecting the stubs based on the contents of __all__ at the second level but that is a bit tedious and error-prone...

@tomvanmele
Copy link
Member

tomvanmele commented Sep 28, 2020

a simpler definition also works

from .mesh import *

class Mesh(BaseMesh):
    pass

however, the following doesn't

from .mesh import *

@tomvanmele
Copy link
Member

the numpy stuff i was talking about

https://github.com/numpy/numpy/blob/master/numpy/__init__.pyi

@yck011522
Copy link
Contributor Author

gentle ping.

@tomvanmele
Copy link
Member

working on it. will send a pr by the end of next week...

@yck011522
Copy link
Contributor Author

Thanks so much. This will be major life saver. 🌈

@tomvanmele
Copy link
Member

solved since version 1.2.0

@yck011522
Copy link
Contributor Author

@brgcode and all who worked on this. Just wanted to say thank you. Worked perfectly with VScode and pylance now. 👍

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

Successfully merging a pull request may close this issue.

4 participants