Josverl / micropython-stubs

Stubs of most MicroPython ports, boards and versions to make writing code that much simpler.
https://micropython-stubs.readthedocs.io
MIT License
148 stars 22 forks source link

u-naming vs non u-unaming problem #761

Closed Omid888 closed 2 weeks ago

Omid888 commented 4 weeks ago

import json & import ujson has the same effect on microcontroller when executed. But in vscode after doing import json the auto completion & its hint-box says you can have the indent parameter for the json.dump() function which is wrong. vscode is referencing to python's json package not the micropython's json package. This bug wasted an hour of me, so I reported to free other folks facing this problem.

Now that u-naming is going obsolete (source) then micropico should first search in its libraries and then go to the general python ones.

P.S: Microcontroller reports this error when json.dump(f,d,indent=4) function is called: extra keyword arguments given

Josverl commented 4 weeks ago

Thanks for the report

Ive been chipping away to improve the static type checking. For stdlib modules such as json this is tedious, as I want to avoid needing to recreate all of typeshed again.

Are you aware that you need a separate configuration setting to allow the stdlib stubs to be used/typeshed to be overridden ? ( I think MicroPico sets this by default , but a sanity check never hurts)

Note that in my testing VSCode / Pylance does provide a correct error for your example, so please check the above setting

image Error : No parameter named "indent" Pylance (reportCallIssue)

I'm testing the stubs against py right and mypy using snippet test.

I don't have explicit tests for json apparently - json is used in a few other samples I use for quality though. I will add a few more examples / test cases just to be sure.

Omid888 commented 3 weeks ago

You're right. This is my configuration:

{
    "python.linting.enabled": true,
    "python.languageServer": "Pylance",
    "python.analysis.typeCheckingMode": "basic",
    "python.analysis.diagnosticSeverityOverrides": {
        "reportMissingModuleSource": "none"
    },
    "micropico.syncFolder": "",
    "micropico.openOnStart": true,
    "python.analysis.typeshedPaths": [
        ".vscode/Pico-W-Stub",
        "~/.micropico-stubs/included"
    ],
    "python.analysis.extraPaths": [
        ".vscode/Pico-W-Stub",
        "~/.micropico-stubs/included"
    ]
}

it's the micropico's default configuration. Is there anything I could change?

Josverl commented 3 weeks ago

Looks ok, Is there a folder named stdlib in .vscode/Pico-W-Stub?

Omid888 commented 3 weeks ago

No, It's empty. stdlib and some other .pyi files exist in "~/.micropico-stubs/included" folder.

P.S: I use macOS. Is this behavior intentional?

Josverl commented 3 weeks ago

Micropy includes the stubs and configuration, in slightly different locations than I use, but the concepts are the same.

Usually this works just fine with micropy, also the os should not matter.

@paulober , any suggestions?

paulober commented 3 weeks ago

The .micropico-stubs/included folder contains the micropython-rp2-rpi_pico_w-stubs package.

@Omid888 I guess I know where to problem is. Have you created the project a while ago? There where some changes in the location where the subs life. Your stubs configuration still included a path to ".vscode/Pico-W-Stub", which no longer exists. So please try removing these two lines from your settings.json.

The issue is, that Pylance just silently fail if they encounter a entry in these settings which does not exist and stop parsing the setting.

paulober commented 3 weeks ago

@Josverl Do you know if python.analysis.stubPath is a replacement for python.analysis.typeshedPaths? As I can't find the typeshedPaths setting in the docs anymore but the stubPath setting is now described. https://code.visualstudio.com/docs/python/settings-reference#_python-language-server-settings

Omid888 commented 3 weeks ago

@paulober Your suggestion (removing those two lines) was the correct solution. Thank You. Now the hint-box shows the correct parameters and the syntax checker throws an error when I use indent parameter (which is correct). ‌But there is a little issue, when I right click on json.dump() and select go to defenition (F12), it doesn't open micropython stubs & instead opens the python's json package.

P.S 1: Isn't it weird that when pylance can't resolve the first option it skips the next configurations? P.S 2: Should I reinstall micropico in a new virtual-env for any other possible changes? (for problems like the current issue) P.S 3: vscode warns that python.analysis.stubsPath does not exist with this error: Unknown Configuration Setting. The correct spelling is python.analysis.stubPath. And it's value should not be a list, but a single string.

paulober commented 3 weeks ago

But there is a little issue, when I right click on json.dump() and select go to defenition (F12), it doesn't open micropython stubs & instead opens the python's json package.

As I was told in this issue. This is by design. If you want to go to the stubs you need to open the context menu and select "Go to declaration".

P.S 1: Isn't it weird that when pylance can't resolve the first option it skips the next configurations?

I totally agree, thats why I opened an issue at the Pylance repo: https://github.com/microsoft/pylance-release/issues/6266

P.S 2: Should I reinstall micropico in a new virtual-env for any other possible changes? (for problems like the current issue)

No you don't need to do that. In general I try to cover these changes by automatically converting old configurations to their new equivalent. But it seems like I forgot to included some for this change or I removed the conversion to early.

P.S 3: vscode warns that python.analysis.stubsPath does not exist with this error: Unknown Configuration Setting. The correct spelling is python.analysis.stubPath. And it's value should not be a list, but a single string.

I misspelled the setting name. And yes it's now a single string. I did not find the typeshedPaths anymore in the settings and this one was new, so maybe it's a replacement and they still support the typeshedPaths one for some time.

Omid888 commented 3 weeks ago

@paulober

This is by design

It's okay. but could you please add this to the documentations (e.g the micropico github readme.md file)? It's not something people could guess, or do trial & error method to find. And it's very important (at least to me).

No you don't need to do that.

Thank you for informing us & and your excellent try to make the plugin better.

I did not find the typeshedPaths anymore

Yes, it's gone and the stub's documentions can be updated to produce less incompatibilities in future, dear @Josverl.

Josverl commented 3 weeks ago

Yes, I just did an overhaul of the docs, and I'll update this as well. This is what I have now I'll drop a note here after I update from this conversation so you can sanity-check.

I may as well add a page with a sample of the micropy settings and stub locations.

Josverl commented 3 weeks ago

python.analysis.stubPath is something different from python.analysis.typeshedPaths

TL;DR. To get the correct time and other stdlib modules for micropython, you must specify exactly one folder in typeshedPaths

Josverl commented 3 weeks ago

Tacking on a sub discussion on the MicroPico configuration, as there are a few things I notice.

{
    "python.linting.enabled": true,  // Old , just remove it 
    "python.languageServer": "Pylance",
    "python.analysis.typeCheckingMode": "basic",
    "python.analysis.diagnosticSeverityOverrides": {
        "reportMissingModuleSource": "none"
    },
    "python.analysis.autoSearchPaths": true,
    "python.analysis.stubPath": "typings",
    "python.analysis.typeshedPaths": [
        "~/.micropico-stubs/included"
    ],
    "python.analysis.extraPaths": [
        "~/.micropico-stubs/included",

    ],
    // MicroPico settings
    "micropico.syncFolder": "",
    "micropico.openOnStart": true,
}
Omid888 commented 2 weeks ago

dear @Josverl and @paulober Thank you both for help.

paulober commented 1 day ago
"python.analysis.stubPath": "typings",

@Josverl So you think I should still add the stubPath setting to the default settings with the value typings or the root of the stubs where the __builtins__.pyi is?