Update coco.py to not utilize os.system#2489
Update coco.py to not utilize os.system#2489arav-agarwal2 wants to merge 7 commits intomlcommons:masterfrom
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
|
@arav-agarwal2 Thank you for the change. Can this code be made Windows compatible as well? Since we are in code freeze window this will likely be merged only after the inference submission deadline. |
|
As it currently stands, I'd wait on getting Windows support unless there's present need to do so. Between the fact that I know most Windows ML-facing devs use WSL or another compatibility layer and the fact that running coco.py using the README's instructions involves running a bash script, I'm not sure how much demand there is. I'm totally happy to wait; I'm working on a variety of code quality updates so as long as it's okay for me to make more PRs, I'm more than ok with them sitting un-merged. |
|
I don't think there's much demand for Windows - but most of the vision MLPerf code works on Windows. And if it's just a 5 minutes change it's worth it as the code is in Python. |
|
Sorry for the delay - I had to spend a little more time than I'd like to get the paths OS-agnostic using pathlib, but it should be working now. |
|
Thanks a lot @arav-agarwal2 We can get this merged in two weeks once submissions are done. Since you're trying to make the code better, copilot can be useful in saving time. |
|
issue #2502 |
Remove potentially dangerous calls to os.system in favor of safer shutil/requests based alternatives.