-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Restored beta warn checks #7282
Conversation
torchvision.disable_beta_transforms_warning() | ||
|
||
from common_utils import CUDA_NOT_AVAILABLE_MSG, IN_FBCODE, IN_OSS_CI, IN_RE_WORKER, OSS_CI_GPU_NO_CUDA_MSG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents warnings from v2 imported in common_utils
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh shoot, thanks Victor for catching that. LGTM, but lint CI is failing: https://app.circleci.com/pipelines/github/pytorch/vision/23407/workflows/1cb0d350-44cd-42b0-a402-e7c63c96cd87/jobs/1835063
Flaky test here (Unit-tests on Linux CPU / tests (3.9) / linux-job (pull_request)):
|
Instead of adding more ad-hoc checks on a separate test suite, I'd prefer going for a more robust solution which would allow to test more scenarios, as the one proposed in #7269. I'll try to open a PR today |
I do not think it worth adding new code like https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/utils/_testing.py#L747 in torchvision just to run some files like If you point is about using again pytest framework instead of manually coding |
We need a lot more checks than those written in this PR. Each of these additional checks would require a separate script to properly test it without risking side-effects. This is untractable. The testing util I want to introduce doesn't have to be as complicated as the one in scikit-learn, we can trim it down for it to look more like https://github.com/pytorch/hub/blob/a8c0f7ccfde4b8ea6cda812f5f61dd82bdeeda65/test_run_python_code.py#L35-L41 |
Closing this since it is superseded by #7288 and the latter was merged. |
Context:
pytest --noconftest
and test is passing again, but I decided to get rid of pytest due to other possible side-effects.