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

Python Analysis Inlay Hints Call Argument Names gives useless and confusing names #4543

Closed
amogorkon opened this issue Jun 26, 2023 · 13 comments
Assignees
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@amogorkon
Copy link

amogorkon commented Jun 26, 2023

Type: Bug

Behaviour

Expected vs. Actual

python.analysis.inlayHints.callArgumentNames enabled produces names as argument names like __iter1/A or __obj/data that are utterly meaningless to the user. I suggest to ignore all name-suggestions that start with __, since those also usually have no documentation available to the user.

Screenshot: https://capture.dropbox.com/kMEbCpqMdPJnM4Ta

Diagnostic data

  • Python version (& distribution if applicable, e.g. Anaconda): 3.11.3
  • Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Global
  • Value of the python.languageServer setting: Pylance

User Settings


languageServer: "Pylance"

formatting
• provider: "black"
• blackPath: "<placeholder>"

testing
• pytestArgs: "<placeholder>"
• pytestEnabled: true

Extension version: 2023.11.11741005
VS Code version: Code - Insiders 1.80.0-insider (b4952d14a465572e33f0635ef58de089227b8876, 2023-06-26T05:32:56.741Z)
OS version: Windows_NT x64 10.0.19045
Modes:

System Info
Item Value
CPUs Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz (8 x 3392)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: unavailable_off
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled
Load (avg) undefined
Memory (System) 31.89GB (10.48GB free)
Process Argv --crash-reporter-id 7b445a4e-4166-4ccb-a3b3-ea07a895bf20
Screen Reader no
VM 0%
A/B Experiments
vsliv695:30137379
vsins829:30139715
vsliv368:30146709
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
vslsvsres303:30308271
pythontb:30258533
pythonptprofiler:30281269
vshan820:30294714
vscod805cf:30301675
bridge0708:30335490
bridge0723:30353136
vsaa593cf:30376535
pythonvs932:30404738
py29gd2263cf:30773604
vsclangdf:30492506
c4g48928:30535728
dsvsc012:30540252
pynewext54:30618038
pylantcb52:30590116
pyind779:30611226
pythonsymbol12:30651887
showlangstatbar:30737417
pythonms35:30671666
03d35959:30757351
ecj1e332:30687743
pythonfmttext:30716741
pythoncmvfstr:30726892
f8hc8238:30694864
fixshowwlkth:30771523
showindicator:30766888
pythongtdpath:30726887
gsof1:30771514
dh2dc718:30770000
pythonnosmt12:30773574
pythonidxpt:30768918
pythondjangots:30768917
pythonnoceb:30766855
copilotsettingt:30767686
e537b577:30772214

@karthiknadig karthiknadig transferred this issue from microsoft/vscode-python Jun 26, 2023
@heejaechang
Copy link
Contributor

tagging @bschnurr

@bschnurr bschnurr assigned bschnurr and unassigned heejaechang Jun 27, 2023
@bschnurr bschnurr added the enhancement New feature or request label Jun 27, 2023
@higorc-mck
Copy link

higorc-mck commented Jul 4, 2023

+1, if a parameter is positional only it shouldn't need a parameter name hint to go with it (since it can't even be used in the first place 😄). Only regular positional/keyword parameters should be hinted, as I see. Thanks for raising this up @amogorkon 🚀

@NoahELE
Copy link

NoahELE commented Jul 12, 2023

I think another way is to trim all the leading underscores, the C/C++ extension does this.

The corresponding setting is C_Cpp.inlayHints.parameterNames.hideLeadingUnderscores, which is enabled by default.

@amogorkon
Copy link
Author

I think another way is to trim all the leading underscores, the C/C++ extension does this.

The corresponding setting is C_Cpp.inlayHints.parameterNames.hideLeadingUnderscores, which is enabled by default.

However, in Python leading underscores have special meaning. A single leading underscore usually means that the name of the argument is not part of the puplic API, so usually there's no documentation for those arguments either.
Double leading underscores are name-mangled, so you'd have to handle this:

class Foo:
    def __init__(self, __a=2):
        self.__a = __a

foo = Foo()
print(foo.__a)   # AttributeError: 'Foo' object has no attribute '__a'
print(foo._Foo__a)  # this is the actual attribute!

If you remove the leading underscores in Python, you're changing the meaning, invariably leading to a lot of confusion.

@NoahELE
Copy link

NoahELE commented Jul 12, 2023

I don't really think it makes sense to use double underscore in parameter name. I would only use double underscore for class attributes. I searched and did not find any information about double underscore in parameters.

I would write code like this:

class Foo:
    def __init__(self, a=2):
        self.__a = a

or like this if I really want positional arguments

class Foo:
    def __init__(self, a=2, /):
        self.__a = a

@erictraut
Copy link
Contributor

This is not a bug. Names with double underscores have a specific meaning in Python.

Prior to PEP 570, there was no formal way to indicate that a function accepted positional-only parameters. Double underscores were used as a convention to indicate this. PEP 570 introduced the special / separator parameter. It is still used in cases where code may need to be backward compatible with versions of Python older than 3.8.

@NoahELE
Copy link

NoahELE commented Jul 12, 2023

Could we introduce a option to trim leading underscores in inlay hints since in recent versions of python this is not needed?

@erictraut
Copy link
Contributor

Trimming the leading underscores will result in an incorrect signature because the parameters will no longer be treated as positional-only.

The correct signature could be retained if the double underscores were trimmed and a / placeholder were added at the appropriate place in the signature. That could be done safely if the Python version were 3.8 or higher.

@yuuuxt
Copy link

yuuuxt commented Jul 25, 2023

Feels for commonly used functions the inlay hints are distracting, and I prefer it being hidden:
image

@NoahELE
Copy link

NoahELE commented Aug 3, 2023

I think that hiding double underscore prefixed inlay hints would be the best way.

@guohanli
Copy link

guohanli commented Sep 5, 2023

I think it is more reasonable to provide a configuration item whether to hide parameters starting with double underscores.

@bschnurr bschnurr added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Sep 6, 2023
@rchiodo
Copy link
Contributor

rchiodo commented Sep 6, 2023

This issue has been fixed in prerelease version 2023.9.11, which we've just released. You can find the changelog here: CHANGELOG.md

@rchiodo rchiodo closed this as completed Sep 6, 2023
@rchiodo
Copy link
Contributor

rchiodo commented Sep 6, 2023

The setting now supports 3 values instead of just true/false.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

9 participants