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

stubgen: use sys.version_info, rather than hardcoding 3.6 #10907

Merged
merged 1 commit into from Nov 21, 2021

Conversation

hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Aug 1, 2021

Not too familiar with stubgen, so maybe there's a good reason for
hardcoding, e.g. to ensure stubs capture sys.version checks in the
source? Even if that's the case, most stubs in typeshed use
sys.version checks for a) checking Python 2, b) if they're stdlib
backports. So hopefully this is a reasonable change.

Fixes #10905, fixes #10791

Not too familiar with stubgen, so maybe there's a good reason for
hardcoding, e.g. to ensure stubs capture sys.version checks in the
source? Most stubs in typeshed use sys.version checks for a) checking
Python 2, b) if they're stdlib backports. So hopefully this is a
reasonable change.

Fixes python#10905
@JelleZijlstra
Copy link
Member

I feel like there should be a command-line option to stubgen for this, but using the runtime version is going to be a better experience than just hardcoding 3.6.

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 1, 2021

You can already pass a Python executable for stubgen to use, and since stubgen will by default try to import things I feel like it is reasonable to require a Python executable of the version you want present on the system.

@braindevices
Copy link

@ethanhs so far --python-executable will not affect the pyversion in stubgen, it is hard coded in defaults.py, and there is no other way to configure that value

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 6, 2021

Right, but if this change is merged then it will use sys.version_info, and so a separate --python-version flag seems unnecessary to me, since you want to be using the version of Python that is importing your code.

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Aug 7, 2021

Note that there isn't a way to parse debug f-strings (the concern of all the issue reports) with Python 3.7 at all, i.e., even for mypy proper we require that if you want to check Python 3.x code when x > 7 you need at least Python 3.x

@braindevices
Copy link

Right, but if this change is merged then it will use sys.version_info, and so a separate --python-version flag seems unnecessary to me, since you want to be using the version of Python that is importing your code.

What will happen if one want to create stub target to py3.6 on a system only have py3.9?
Is the stub somehow sensitive to python3 versions? If not it should be fine. If yes, there need to be a cmd argument for that.

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 7, 2021

Yes, but if stubgen is trying to import things from the Python code, it must use the correct Python version anyway, otherwise things are going to be incorrect.

@eelkevdbos
Copy link

What are the next steps in getting this merged / released? The solution proposed by @hauntsaninja feels pretty solid. Are there any objections blocking? I feel like the other solution is to use sys.version_info[:2] as a default value for the --python-executable option -- even if it would result in errors because of the incompatibility in python version and stubbed code.

@JelleZijlstra JelleZijlstra merged commit 4dfa38c into python:master Nov 21, 2021
@JelleZijlstra
Copy link
Member

I merged it, but releasing is not in my control.

@hauntsaninja hauntsaninja deleted the stubg branch November 28, 2021 07:26
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
Not too familiar with stubgen, so maybe there's a good reason for
hardcoding, e.g. to ensure stubs capture sys.version checks in the
source? Most stubs in typeshed use sys.version checks for a) checking
Python 2, b) if they're stdlib backports. So hopefully this is a
reasonable change.

Fixes python#10905
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants