Remove macOS stderr suppression

pytorch-labs / torchfix

TorchFix - a linter for PyTorch-using code with autofix support

Other

98 stars 16 forks source link

Remove macOS stderr suppression #76

Open sbrugman opened 1 month ago

sbrugman commented 1 month ago

contextlib.redirect_stderr works on Mac, no need to additional clause

sbrugman commented 1 month ago

We actually had a subtle bug in the previous implementation: when --show-stderr was provided, the program would exit because None is not a valid contextmanager. The contextlib.nullcontext is an elegant alternative for the time being.

Changes:

FIX: avoid crash when --show-stderr is passed
ENH: remove obviated dup2 special suppression for macOS
CI: Increase verbosity of pytest for more informative errors.
CI: Test the torchfix binary is available (used by the test)
TST: Add a test for --show-stderr

kit1980 commented 1 month ago

Thanks! I'll double check on an x86 Mac where the issue was happening.

And I need to follow-up on https://github.com/Instagram/LibCST/issues/944 eventually to fix the root cause...

sbrugman commented 1 month ago

@kit1980 Github Actions also offers macOS x86 runners - I've added that one and the test pass. Windows is also included now.

kit1980 commented 1 month ago

I tried this on my x86 macbook, and the suppression doesn't work. tests/test_torchfix.py::test_stderr_suppression actually passed, but running torchfix . causes tons of noise:

torchfix .
Failed to determine module name for /Users/sdym/repos/pytorch-release/vision/examples/cpp/script_model.py: '/Users/sdym/repos/pytorch-release/vision/examples/cpp/script_model.py' is not in the subpath of '' OR one path is relative and the other is absolute. Failed to determine module name for /Users/sdym/repos/pytorch-release/vision/.github/process_commit.py: '/Users/sdym/repos/pytorch-release/vision/.github/process_commit.py' is not in the subpath of '' OR one path is relative and the other is absolute.

sbrugman commented 1 month ago

Pity, that version of Mac then probably outputs the stderr to stdout and leaves stderr empty...