Skip to content

[bugfix] add 'tabulate' to the requirements.txt since the CLI cmd_list uses it #627

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

Closed
wants to merge 1 commit into from

Conversation

kiukchung
Copy link
Contributor

@kiukchung kiukchung commented Oct 26, 2022

Fixes a bug introduced in https://github.com/pytorch/torchx/pull/579/files#diff-0aff65b8fe78257a723972926d63b801c495e54669248db41bed7536055aad92

Using tabulate in cmd_list.py but not adding it to requirements.txt causes torchx CLI to fail:

$ torchx runopts aws_batch

Traceback (most recent call last):
  File "/home/ubuntu/.pyenv/versions/venv39/bin/torchx", line 5, in <module>
    from torchx.cli.main import main
  File "/home/ubuntu/.pyenv/versions/3.9.13/envs/venv39/lib/python3.9/site-packages/torchx/cli/main.py", line 17, in <module>
    from torchx.cli.cmd_list import CmdList
  File "/home/ubuntu/.pyenv/versions/3.9.13/envs/venv39/lib/python3.9/site-packages/torchx/cli/cmd_list.py", line 11, in <module>
    from tabulate import tabulate
ModuleNotFoundError: No module named 'tabulate'

Copy link
Member

@d4l3k d4l3k left a comment

Choose a reason for hiding this comment

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

thanks for the fix!

@d4l3k
Copy link
Member

d4l3k commented Oct 26, 2022

@kiukchung can you sign the new CLA?

@codecov
Copy link

codecov bot commented Oct 26, 2022

Codecov Report

Merging #627 (940b96d) into main (f61f22c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #627   +/-   ##
=======================================
  Coverage   94.86%   94.86%           
=======================================
  Files          69       69           
  Lines        4832     4832           
=======================================
  Hits         4584     4584           
  Misses        248      248           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants