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

[Bug Report] VSCode PyLance stubs are outdated/incorrect #2918

Closed
BlueskyFR opened this issue Jun 22, 2022 · 14 comments
Closed

[Bug Report] VSCode PyLance stubs are outdated/incorrect #2918

BlueskyFR opened this issue Jun 22, 2022 · 14 comments

Comments

@BlueskyFR
Copy link

BlueskyFR commented Jun 22, 2022

Hi!

VSCode's PyLance (Python language server) extension contains some builtin stubs, with a gym-stubs directory that contains the following line for instance:

class Env(object):
    metadata: Dict[str, List[str]]

It is of course wrong since metadata can contain render_fps which is of type int.

Who/where are those stubs managed? They should be updated since VSCode reports errors in code analysis in the meantime.

Please let me know if you have any question.

@RedTachyon
Copy link
Contributor

This seems like a PyLance issue? We don't provide any explicit stubs, just type annotations in most places.

What might maybe perhaps work is adding an explicit type hint in the Env base class like metadata: dict[str, Any] = {"render_modes": []}, but I wouldn't even be able to check if it works

Scratch that, it definitely is a PyLance issue. They seem to maintain stubs in a special repo which seems kinda... insane unmaintainable.
I raised an issue in the repo that I suspect is the culprit, if you have any extra information you can provide there, that would probably be helpful. But ultimately I'm pretty sure there's nothing we can do, and it's up to the PyLance team

@BlueskyFR
Copy link
Author

Well, maintaining stubs in another non-openai repo seems like a really bad idea, maybe they should just remove them and let you type your stuff

@pseudo-rnd-thoughts
Copy link
Contributor

Given that we (the volunteer dev group) have only been doing this for about a year then I suspect that the stub was created when there were no type hints in the project

@BlueskyFR
Copy link
Author

Given that we (the volunteer dev group) have only been doing this for about a year then I suspect that the stub was created when there were no type hints in the project

Is Gym a project you contribute to on your free time or are you "assigned" to it? Just curious 😉

@pseudo-rnd-thoughts
Copy link
Contributor

Join the discord (https://discord.gg/nHg2JRN489) and we ask for developers to help with stuff when problems occur

@RedTachyon
Copy link
Contributor

@BlueskyFR

Is Gym a project you contribute to on your free time or are you "assigned" to it? Just curious 😉

Also for posterity, none of us are affiliated with OpenAI, I feel like this might be a misunderstanding. We're not assigned to it because there's nobody who'd be able to assign us anywhere

@BlueskyFR
Copy link
Author

Thanks! I though OpenAI was actually maintaining this ;)

@bschnurr
Copy link

bschnurr commented Jun 28, 2022

gym/gym/core.py

Line 106 in 0dba072

metadata = {"render_modes": []}

Not sure what the type of metadata should be, but it should still have a typeAnnoation
something like:
metadata: dict[str, List[str|float]] = {"render_modes": []}

or
metadata: dict[str, List[Any]] = {"render_modes": []}

@bschnurr
Copy link

also there is a way to test of type completeness on your public api using pyright

pip install gym
pip install pyright

pyright --verifytypes gym

Results:

Symbols exported by "gym": 1371
With known type: 324
With ambiguous type: 196
With unknown type: 851
Functions without docstring: 283
Functions without default param: 329
Classes without docstring: 25

Other symbols referenced but not exported by "gym": 880
With known type: 805
With ambiguous type: 0
With unknown type: 75

Type completeness score: 23.6%

can also integrate a github action like typeshed to run pyright
https://github.com/python/typeshed/blob/f69f711d5a08bbc3aeebdc9e2483f13d3a6138aa/.github/workflows/tests.yml#L79

@pseudo-rnd-thoughts
Copy link
Contributor

pseudo-rnd-thoughts commented Jun 29, 2022

@bschnurr Thanks for the suggestion, I wasn't aware of the --verifytypes argument, that is very helpful
Complete type hinting is a plan for the project, however, we have been primarily making changes to the api current so waiting for that to stabilise to then hint the project
We have already implemented pyright in our pre-commit hooks here however as you can see a majority of the folders are excluded from the check as we were planning on slowly type hinting the project.
Is it better to remove the pyright type stubs for gym and let us slowly add them to the project?

@bschnurr
Copy link

bschnurr commented Jun 29, 2022

If we detect a py.typed file we shouldn't be using our shipped stubs.. i'll double check whats happening

@bschnurr
Copy link

bschnurr commented Jun 29, 2022

confirmed. when using a newer version of gym with py.typed file we dont use our stubs.

also it appears we are inferring the type of metadata as 'dict[str,list]'
image

new 'inlayHint' setting turned on

@pseudo-rnd-thoughts
Copy link
Contributor

pseudo-rnd-thoughts commented Jul 1, 2022

@bschnurr Currently there is a weird bug occurring with these type stubs, I can move the discussion to the pyright repo if that is more helpful.
Im currently making the core API follow the pyright strict requirements
However from gym.logger import warn, deprecation fails on deprecation as even though logger contains the deprecation it is not included in the type stubs.
I discovered this after an hour, by commenting out the warn and vscode points to the gym type stub which contains warn but not deprecation function.
Therefore, my suspicion is that for some reason, vscode is favoring the type stub definition that doesn't exist the actual definition that the function is pointing to

System

Edit: Looking at this on my Macbook then this issue no longer happens

@jkterry1
Copy link
Collaborator

jkterry1 commented Jul 6, 2022

Hey, I'm going to close this as the issue has been resolved with the new type hinting additions in master, please let us know if you need any additional help

@jkterry1 jkterry1 closed this as completed Jul 6, 2022
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

5 participants