-
Notifications
You must be signed in to change notification settings - Fork 670
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
(feat) Make tqdm progress reporting opt-in #1741
Conversation
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.
I'm curious if there's a way we can disable it via an environment variable if tqdm
is installed. The problem is that I don't like the idea of "if you happen to have this thing installed we'll pollute your stdout".
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@LeonLuttenberger When you call tqdm there is I wouldn't put any extra safety checks, IMO one mechanism by which the user can control this behavior is enough. Especially if it's more development time concern (in production your environments should only contain what is necessary). |
@LeonLuttenberger also disabling by env variable wouldn't be opt-in. Imagine having to install an unnecessary dependency on the workers of your cluster only then to disable in via an env variable. |
That's a good point. I was hoping there would be an environment variable in Ray that we could plug into easily, but you're right, it's not a big deal either way. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
tqdm
progress bar should be opt-in instead of installed with ray extra by default. It's only required when a user expects to run the script and observe live output, otherwise it's polluting stderr.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.