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

[CI] mypy install missing stubs #4014

Merged
merged 9 commits into from
Jun 9, 2021
Merged

Conversation

AnirudhDagar
Copy link
Contributor

This PR is a follow up to #4011 (see for more details) and fixes failing python type check CI #4009 by installing the missing stubs.

I've not reverted the change in #4011, let me know if that is recommended. Thank you :))

@datumbox
Copy link
Contributor

datumbox commented Jun 9, 2021

@AnirudhDagar feel free to revert the ignoring to see if your update works and solve this properly.

Note that the CI is complaining because the change is not present on config.yml.in

.circleci/config.yml Outdated Show resolved Hide resolved
@AnirudhDagar
Copy link
Contributor Author

Thanks @datumbox, will fix that.

@AnirudhDagar
Copy link
Contributor Author

AnirudhDagar commented Jun 9, 2021

We need to pass y, currently, the command sits and waits for that input. Any suggestions for the same?
mypy --install-types doesn't support a -y option to automatically do that.

EDIT: I'm thinking of yes | mypy --install-types @datumbox LMK

@AnirudhDagar
Copy link
Contributor Author

@datumbox could you please help me here. Thanks in advance!

Since the output of mypy --install-types raises an error the CI automatically fails with that, without waiting for it to be fixed by installing the required stubs below

This is the output on my local machine but CI exits as soon as the error is raised.

~/Desktop/Work/vision fix-mypy ❯ yes | mypy --install-types                                                                           

torchvision/datasets/utils.py:204: error: Library stubs not installed for "requests" (or incompatible with Python 3.8)  [import]
        import requests
    ^
torchvision/datasets/utils.py:204: note: Hint: "python3 -m pip install types-requests"
torchvision/datasets/utils.py:204: note: (or run "mypy --install-types" to install all missing stub packages)
torchvision/datasets/utils.py:204: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 110 source files)

Installing missing stub packages:
/Users/gollum/anaconda3/envs/vision/bin/python -m pip install types-requests

Install? [yN]
Collecting types-requests
  Using cached types_requests-0.1.8-py2.py3-none-any.whl (22 kB)
Installing collected packages: types-requests
Successfully installed types-requests-0.1.8

@AnirudhDagar
Copy link
Contributor Author

The other option to fix this is more specific here. We can do pip install --user --progress-bar off types-requests instead of yes | mypy --install-types. I'll update the PR to see if that works.

@datumbox
Copy link
Contributor

datumbox commented Jun 9, 2021

@AnirudhDagar I didn't intervene because you are doing all the checks and workarounds I would be doing as well. I'm not too familiar with all the changes that mypy did on the last version yesterday (part of the reason why I wanted a quick fix on #4011).

It's unclear to me why it's not blocking for install but hopefully your last modification will work. Let's see.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@AnirudhDagar
Copy link
Contributor Author

@datumbox Yeah I completely understand that a fast fix was required at the moment. Thank you :))

@NicolasHug NicolasHug merged commit 9ff01a0 into pytorch:master Jun 9, 2021
@NicolasHug
Copy link
Member

Thanks @AnirudhDagar !

@AnirudhDagar AnirudhDagar deleted the fix-mypy branch June 9, 2021 10:22
facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2021
Reviewed By: fmassa

Differential Revision: D29097719

fbshipit-source-id: 9dc4d7324b28dbbae1f3fb24e433582fd653d09d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants