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

Intellisense suggests imported names which should be considered an implementation detail #1875

Closed
parched opened this issue Sep 26, 2021 · 8 comments
Assignees
Labels
needs decision Do we want this enhancement?

Comments

@parched
Copy link

parched commented Sep 26, 2021

PEP-8 mentions

Imported names should always be considered an implementation detail

When I have

# b.py
import c

x = 1

and I create

# a.py
import b
b. # intellisense shows c and x, but c should be considered an implementation detail

I don't actually mind that c is shown, because I expect that some code doesn't consider imports an implementation detail, e.g. __init__.py, but I think it would be more useful if all the imported names were at the bottom of the list.

Perhaps the order could be something like this:

  • Everything in __all__
  • All non imported names
  • All imported names

Even better would be if the names intellisense thinks might be private are greyed slightly (It could be a good way to solve microsoft/python-language-server#619)

Pylance version: Pylance language server 2021.9.3 (pyright d2771b18)
OS version: WIndows 10
Python version: 3.8.10

@erictraut
Copy link
Contributor

The Intellisense mechanism in pylance does enforce this convention for "py.typed" libraries and for type stubs. In these cases, it does not offer these imported symbols as completion suggestions unless they are included in __all__ or use a redundant aliased form of import (e.g. from x import y as y) as documented in PEP 484. For more details about which library symbols are considered private by pylance, please refer to this documentation.

While PEP 8 does suggest that imported names should be considered an implementation detail, we've found that most libraries (especially those that are not "py.typed") do not follow this convention. If we were to enforce this convention more generally, I can assure you that we'd receive a deluge of bug reports from unhappy pylance users!

@parched
Copy link
Author

parched commented Sep 27, 2021

Ok, thanks for the link, I was not aware of "py.typed". I've added it to my package but it doesn't seem to help, I assume that's because it only works inter-package, my use case is two modules in the same package.

Given that most libraries don't follow that convention, would it be fine to just make imported names a lower priority in the list of suggestions?

@erictraut
Copy link
Contributor

Yes, "py.typed" applies only to packages installed within your Python environment.

We will discuss your suggestion, but my educated guess is that most pylance users will not want us to deprioritize these symbols in the suggestion list. We might be able to validate (or refute) that hypothesis through an experiment using telemetry-based feedback.

It's unfortunate that Python doesn't have an export keyword or some other way of specifying explicitly which symbols are intended to be exported. Other languages like TypeScript have this support.

@jakebailey
Copy link
Member

Regardless of the prioritization, I think it would make sense to also consider user-code with py.typed to be py.typed like we do libraries; that's how I thought it was already. Otherwise, one editing a py.typed library won't know that they are writing code that is mistakenly incompatible with importers.

@judej judej added the needs investigation Could be an issue - needs investigation label Sep 27, 2021
@github-actions github-actions bot removed the triage label Sep 27, 2021
@smackesey
Copy link

smackesey commented Nov 21, 2021

@erictraut It would be nice if there were an option where users could either:

  1. specify packages for which Pylance's default "export inference" rules could be overridden
  2. treat imported names as public if they are inside an __init__.py.

This is because your reasoning here is still true even of library authors who include py.typed:

While PEP 8 does suggest that imported names should be considered an implementation detail, we've found that most libraries (especially those that are not "py.typed") do not follow this convention. If we were to enforce this convention more generally, I can assure you that we'd receive a deluge of bug reports from unhappy pylance users!

This is probably because PEP 561 (which defines py.typed and is the official reference for how to package type information) says nothing about __all__ or privacy. So for many it is a complete surprise that Pylance modulates privacy rules based on the presence of py.typed. As a result, there are nicely-typed libraries out there for which Pylance hides almost all of the (intended) public interface from autocomplete.

Also, PEP8 isn't really clear that imported names in __init__.py are private (emphasis mine):

To better support introspection, modules should explicitly declare the names in their public API using the all attribute. Setting all to an empty list indicates that the module has no public API.

Even with all set appropriately, internal interfaces (packages, modules, classes, functions, attributes or other names) should still be prefixed with a single leading underscore.

An interface is also considered internal if any containing namespace (package, module or class) is considered internal.

Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module's API, such as os.path or a package's init module that exposes functionality from submodules.

@heejaechang heejaechang self-assigned this Apr 13, 2022
@heejaechang heejaechang added needs decision Do we want this enhancement? and removed needs investigation Could be an issue - needs investigation labels Apr 18, 2022
@judej
Copy link
Contributor

judej commented Apr 27, 2022

We do enforce this for type stubs and py.typed libraries, the practice of reexporting imported symbols is too common in most Python code for us to enforce this.

@judej judej closed this as completed Apr 27, 2022
@parched
Copy link
Author

parched commented Feb 7, 2023

@judej @erictraut coming back to this, I'm finding my team is doing things like this

from x import thing as _thing

Is that the Pylance recommended workaround?

Is it really a problem to put the imported names at the bottom of the intellisense list? Even if it's just for py.typed projects?

@LagSlug
Copy link

LagSlug commented Sep 24, 2023

I'm new to python, and it would be very useful to be able to filter out these implementation details from Intellisense autocomplete, even if it's a setting I have to opt into. Such as it is I'm resorting to what seems like a step backwards in code quality.

I would like to know what is preferred, because I tend to make a lot of modules with just a few or even just one method being intentionally exported, and I just dislike all of the clutter that comes with seeing everything that has been declared in the auto completion list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision Do we want this enhancement?
Projects
None yet
Development

No branches or pull requests

7 participants