-
Notifications
You must be signed in to change notification settings - Fork 26.3k
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
don't complain about missing W&B when WANDB_DISABLED=true #6036
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6036 +/- ##
==========================================
- Coverage 78.50% 77.31% -1.19%
==========================================
Files 146 146
Lines 26249 26251 +2
==========================================
- Hits 20606 20297 -309
- Misses 5643 5954 +311
Continue to review full report at Codecov.
|
LGTM |
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.
Two nits, but looks good to me and is a useful feature, thanks!
src/transformers/trainer.py
Outdated
"You are instantiating a Trainer but W&B is not installed. To use wandb logging, " | ||
"run `pip install wandb; wandb login` see https://docs.wandb.com/huggingface." | ||
) | ||
if os.environ.get("WANDB_DISABLED") != "true": |
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.
Synthesize in a elif
?
src/transformers/trainer_tf.py
Outdated
"You are instantiating a Trainer but W&B is not installed. To use wandb logging, " | ||
"run `pip install wandb; wandb login` see https://docs.wandb.com/huggingface." | ||
) | ||
if os.environ.get("WANDB_DISABLED") != "true": |
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.
Same here
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.
Thank you, @sgugger, for the suggestions - done.
Note, the project has a failing test - I did 2 PRs updates yesterday and today - both unrelated to my work another one from last night: Please kindly trigger a re-run of CI for this PR. Thank you. |
Yeah, the CI is flaky sometimes. Re-triggered the failing test. |
When it happens it's the same test that fails - perhaps it could be fixed?
Thank you, @sgugger |
Yeah we try to fix those flaky tests as we catch them. Don't hesitate to report the name and logs on your own too. |
I get a lot of crashes with W&B in transformers examples: #5835 so I have to use
WANDB_DISABLED=true
- this PR removes a complaint that shouldn't be there when this env var is used.