Port NumPy typing testing style to PyTorch by guilhermeleobas · Pull Request #52408 · pytorch/pytorch

guilhermeleobas · 2021-02-18T00:38:41Z

ref: #16574

facebook-github-bot · 2021-02-18T00:38:51Z

💊 CI failures summary and remediations

As of commit 6b42dfa (more details on the Dr. CI page):

1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_windows_vs2019_py36_cuda10.1_build (1/1)

Step: "Install Cuda" (full log | diagnosis details | 🔁 rerun)

ls: cannot access '/c/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe': No such file or directory


Folders: 11
Files: 130
Size:       907512
Compressed: 111420
+ mkdir -p 'C:/Program Files/NVIDIA Corporation/NvToolsExt'
+ cp -r NvToolsExt/bin NvToolsExt/docs NvToolsExt/include NvToolsExt/lib NvToolsExt/samples 'C:/Program Files/NVIDIA Corporation/NvToolsExt/'
+ export 'NVTOOLSEXT_PATH=C:\Program Files\NVIDIA Corporation\NvToolsExt\'
+ NVTOOLSEXT_PATH='C:\Program Files\NVIDIA Corporation\NvToolsExt\'
+ ls '/c/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe'
ls: cannot access '/c/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v10.1/bin/nvcc.exe': No such file or directory
+ echo 'CUDA installation failed'
CUDA installation failed
+ mkdir -p /c/w/build-results
+ 7z a 'c:\w\build-results\cuda_install_logs.7z' cuda_install_logs

7-Zip 19.00 (x64) : Copyright (c) 1999-2018 Igor Pavlov : 2019-02-21

Scanning the drive:
1 folder, 2 files, 3721951 bytes (3635 KiB)

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

rgommers

Thanks @guilhermeleobas, looks great from a first look. I will do a more detailed review later.

Would it make sense to also add a pass/ and a fail/ directory (like NumPy has too) with a couple of tests in each? Or do you prefer to leave that for later.

rgommers · 2021-02-23T21:34:32Z

Cc @malfet and @walterddr for visibility. This PR introduces a new, more compact and extensive, test style for type annotation tests. As suggested in #16574 (comment). The machinery is largely taken from NumPy, this approach is working well there.

malfet · 2021-02-23T22:20:46Z

@guilhermeleobas can you please rebase your change against https://github.com/pytorch/pytorch/tree/viable/strict ? (to see clean picture on the PR)

…to mypy_tests

guilhermeleobas · 2021-02-24T20:14:07Z

Would it make sense to also add a pass/ and a fail/ directory (like NumPy has too) with a couple of tests in each? Or do you prefer to leave that for later.

@rgommers, I can add both in a later PR.

@guilhermeleobas can you please rebase your change against https://github.com/pytorch/pytorch/tree/viable/strict ? (to see clean picture on the PR)

@malfet, done!

codecov · 2021-02-24T23:52:42Z

Codecov Report

Merging #52408 (7cb370e) into master (7a178a8) will decrease coverage by 0.33%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #52408      +/-   ##
==========================================
- Coverage   80.76%   80.42%   -0.34%     
==========================================
  Files        1975     1975              
  Lines      216841   216841              
==========================================
- Hits       175134   174404     -730     
- Misses      41707    42437     +730

facebook-github-bot

@malfet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr

looks good. do we want to add the test into test/run_test.py list and run it on CI?

rgommers · 2021-02-25T16:41:39Z

looks good. do we want to add the test into test/run_test.py list and run it on CI?

Yes, I think so.

Would it make sense to also add a pass/ and a fail/ directory (like NumPy has too) with a couple of tests in each? Or do you prefer to leave that for later.

@rgommers, I can add both in a later PR.

Sounds good. Another thing that would be good as a follow-up is to get rid of everything in test/type_hint_tests/ by converting those to the test style in this PR.

malfet · 2021-02-26T05:48:30Z

Can you please add this new .py file to run_test.py (PyTorch test framework is not pytest compatible yet, so your PR is essentially a no-op at the moment)

guilhermeleobas · 2021-02-26T15:49:07Z

Done @malfet

malfet · 2021-02-26T20:26:30Z

+import shutil
+from typing import IO, Dict, List
+
+import pytest


Please remove pytest dependency (as I've mentioned in my previous comment, pytorch test infra is not compatible with it yet)

Oops, my bad. Do you know if pytest support is landing in a near future?

as far as i know there's no plan. see: https://github.com/pytorch/pytorch/blob/master/CONTRIBUTING.md#unit-testing

Please remove pytest dependency (as I've mentioned in my previous comment, pytorch test infra is not compatible with it yet)

After looking into it, this isn't quite right - there are unconditional import pytest statements in the codebase (see
#11578 (comment)). There's no point spending significant effort rewriting this bit of by now well-tested code. I think the correct solution is to keep this file as is, and:

add the file name to USE_PYTEST_LIST in test/run_test.py

make the test runnable standalone by, at the end of the file, adding:

if __name__ == '__main__': pytest.main([__file__])

Thanks for the suggestion Ralf. I've put the file on USE_PYTEST_LIST

malfet · 2021-02-26T20:31:45Z

+    if os.path.isdir(CACHE_DIR):
+        shutil.rmtree(CACHE_DIR)


Can you add a comment why cache needs to be invalidated before running the tests?
MyPy should be capable of updating the cache if files have changed between the runs, shouldn't it?

This was also inherited from NumPy source code. But from my personal experience, 99% of the time mypy is capable of invalidating the cache. Only in a few specific scenarios this wasn't true.

facebook-github-bot

@malfet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-03-10T20:19:55Z

@malfet merged this pull request in cb68039.

#53167) Summary: This is a follow-up PR of #52408 and move/convert all files under `test/type_hint_tests/*.py` to use the new test style. Pull Request resolved: #53167 Reviewed By: ejguan Differential Revision: D27081041 Pulled By: walterddr fbshipit-source-id: 56508083800a5e12a7af88d095ca26229f0df358

Summary: ref: pytorch#16574 Pull Request resolved: pytorch#52408 Reviewed By: anjali411 Differential Revision: D26654687 Pulled By: malfet fbshipit-source-id: 6feb603d8fb03c2ba2a01468bfde1a9901e889fd

Summary: This is a follow-up PR of #52408 and includes the `pass/` and `fail/` directories. Pull Request resolved: #54234 Reviewed By: walterddr Differential Revision: D27681410 Pulled By: malfet fbshipit-source-id: e6817df77c758f4c1295ea62613106c71cfd3fc3

Summary: This is a follow-up PR of pytorch#52408 and includes the `pass/` and `fail/` directories. Pull Request resolved: pytorch#54234 Reviewed By: walterddr Differential Revision: D27681410 Pulled By: malfet fbshipit-source-id: e6817df77c758f4c1295ea62613106c71cfd3fc3

Summary: ref: pytorch#16574 Pull Request resolved: pytorch#52408 Reviewed By: anjali411 Differential Revision: D26654687 Pulled By: malfet fbshipit-source-id: 6feb603d8fb03c2ba2a01468bfde1a9901e889fd

pytorch#53167) Summary: This is a follow-up PR of pytorch#52408 and move/convert all files under `test/type_hint_tests/*.py` to use the new test style. Pull Request resolved: pytorch#53167 Reviewed By: ejguan Differential Revision: D27081041 Pulled By: walterddr fbshipit-source-id: 56508083800a5e12a7af88d095ca26229f0df358

Summary: This is a follow-up PR of pytorch#52408 and includes the `pass/` and `fail/` directories. Pull Request resolved: pytorch#54234 Reviewed By: walterddr Differential Revision: D27681410 Pulled By: malfet fbshipit-source-id: e6817df77c758f4c1295ea62613106c71cfd3fc3

guilhermeleobas added 3 commits February 13, 2021 19:11

upd

489ce77

Merge branch 'master' of github.com:pytorch/pytorch into mypy_tests

2b97acf

Initial support for numpy like test infrastructure for type annotations

4250ced

guilhermeleobas added the module: typing Related to mypy type annotations label Feb 18, 2021

guilhermeleobas self-assigned this Feb 18, 2021

facebook-github-bot added the cla signed label Feb 18, 2021

guilhermeleobas mentioned this pull request Feb 18, 2021

Make it possible to use mypy to typecheck code that uses PyTorch #16574

Closed

pytorchbot added the open source label Feb 18, 2021

guilhermeleobas requested a review from rgommers February 19, 2021 18:08

guilhermeleobas marked this pull request as ready for review February 19, 2021 18:08

rgommers reviewed Feb 23, 2021

View reviewed changes

Merge branch 'viable/strict' of https://github.com/pytorch/pytorch in…

d275d3d

…to mypy_tests

facebook-github-bot reviewed Feb 25, 2021

View reviewed changes

walterddr approved these changes Feb 25, 2021

View reviewed changes

guilhermeleobas added 2 commits February 26, 2021 15:24

add file to run_test

9283eb4

Merge branch 'master' of github.com:pytorch/pytorch into mypy_tests

b2b9641

malfet reviewed Feb 26, 2021

View reviewed changes

Comment thread test/test_typing.py Outdated

malfet reviewed Feb 26, 2021

View reviewed changes

Comment thread test/test_typing.py Outdated

rgommers mentioned this pull request Feb 28, 2021

Request to import pytest in test/*.py #11578

Open

Move test to USE_PYTEST_LIST

ee34f1e

guilhermeleobas added 4 commits March 1, 2021 20:14

remove numpy dependency

37b89b5

add pytest.main

7cb370e

flake8

d9edd57

flake8

6b42dfa

guilhermeleobas mentioned this pull request Mar 3, 2021

remove type_hint_tests and convert the files to use the new test style #53167

Closed

facebook-github-bot reviewed Mar 5, 2021

View reviewed changes

facebook-github-bot closed this in cb68039 Mar 10, 2021

facebook-github-bot added the Merged label Mar 10, 2021

guilhermeleobas deleted the mypy_tests branch March 15, 2021 17:47

guilhermeleobas mentioned this pull request Mar 18, 2021

Port NumPy typing testing style to PyTorch #54234

Closed

Conversation

guilhermeleobas commented Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_windows_vs2019_py36_cuda10.1_build (1/1)

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

rgommers commented Feb 23, 2021

Uh oh!

malfet commented Feb 23, 2021

Uh oh!

guilhermeleobas commented Feb 24, 2021

Uh oh!

codecov Bot commented Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

rgommers commented Feb 25, 2021

Uh oh!

malfet commented Feb 26, 2021

Uh oh!

guilhermeleobas commented Feb 26, 2021

Uh oh!

malfet Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

walterddr Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

rgommers Feb 28, 2021

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Mar 1, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

malfet Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

guilhermeleobas Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

guilhermeleobas commented Feb 18, 2021 •

edited

Loading

facebook-github-bot commented Feb 18, 2021 •

edited

Loading

codecov Bot commented Feb 24, 2021 •

edited

Loading