Skip to content

Make pytype respect required python versions of stubs #10810

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

Closed
wants to merge 4 commits into from

Conversation

hamdanal
Copy link
Contributor

This is the pytype part of #10722

hamdanal and others added 2 commits October 1, 2023 09:14
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
colors, keep summary at the end, file before error stack, better alignment
@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 1, 2023

I just posted some explanation in #10722 (comment) of the context/history around why we pytype and pyright are run so differently from mypy in CI.

The result of all that is that pytype_test.py now only works if you've already done something like what we do in CI to install all the non-types dependencies for all our stubs packages:

- run: pip install -r requirements-tests.txt
- name: Install external dependencies for 3rd-party stubs
run: |
DEPENDENCIES=$(python tests/get_external_stub_requirements.py)
if [ -n "$DEPENDENCIES" ]; then
echo "Installing packages: $DEPENDENCIES"
pip install $DEPENDENCIES
fi
- run: pip freeze --all
- run: ./tests/pytype_test.py --print-stderr

(This probably isn't an ideal state of affairs; we should probably alter pytype_test.py so that it includes this logic and installs all these packages into a temporary venv, rather than having to run two scripts sequentially in order for the second one to work.)

As a result, I think this PR as it currently stands is insufficient, since tests/get_external_stub_requirements.py is probably still going to include matplotlib>=3.8 in its output after #10721 is merged, and matplotlib>=3.8 can't be installed with Python <3.9. (tests/get_external_stub_requirements.py already requires Python 3.9+ to even run, though; we require Python 3.9+ for all our tests except mypy_test.py, which needs to be compatible with Python 3.8.)

@hamdanal
Copy link
Contributor Author

hamdanal commented Oct 1, 2023

Thanks for the context. I’ll close this now.

@hamdanal hamdanal closed this Oct 1, 2023
@hamdanal hamdanal deleted the pytype-requires-python branch October 1, 2023 21:18
@AlexWaygood
Copy link
Member

No worries. Sorry I wasn't clearer earlier about the "philosophical difference" between this script and the mypy scripts :(

@hamdanal
Copy link
Contributor Author

hamdanal commented Oct 3, 2023

Don’t worry about it. I was happy it worked locally that I forgot to look how it works in CI. I will look into it again in the future when I get some time and if no one else beats me to it.

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

Successfully merging this pull request may close these issues.

3 participants