fix(wandb): allow custom init args #6989
Conversation
|
Hello @borisdayma! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 馃嵒 Comment last updated at 2021-05-04 08:40:14 UTC |
Codecov Report
@@ Coverage Diff @@
## master #6989 +/- ##
========================================
- Coverage 91% 83% -8%
========================================
Files 200 200
Lines 12875 13594 +719
========================================
- Hits 11765 11337 -428
- Misses 1110 2257 +1147 |
borisdayma
requested review from
carmocca,
justusschock,
kaushikb11,
SeanNaren and
williamFalcon
as code owners
borisdayma
changed the title
Borda
added
3rd party
pytorch_lightning/loggers/wandb.py
Outdated
| @@ -89,13 +88,11 @@ class WandbLogger(LightningLoggerBase): | |||
|
|
|||
| def __init__( | |||
| self, | |||
| name: Optional[str] = None, | |||
There was a problem hiding this comment.
this will fail if a user uses positional arguments
pls, maintain a proper deprecation process...
There was a problem hiding this comment.
Ok I聽put back name and project at their previous positions.
pytorch_lightning/loggers/wandb.py
Outdated
| self._kwargs = kwargs | ||
| # set wandb init arguments | ||
| self._wandb_init = dict(resume='allow', id=version or id, dir=save_dir, | ||
| anonymous='allow' if anonymous is True else None if anonymous is False else anonymous) |
There was a problem hiding this comment.
can we rather define this as look-up-table aka dictionary?
an_lut = {True: 'allow', False: None}
anonymous = an_lut.get(anonymous, anonymous)
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need further help see our docs: https://pytorch-lightning.readthedocs.io/en/latest/generated/CONTRIBUTING.html#pull-request or ask the assistance of a core contributor here or on Slack. Thank you for your contributions. |
|
Hi, any update on this PR? |
There was a problem hiding this comment.
LGTM
I think this should go into 1.3 because it's really more a fix than a feature.
Changed milestone. Thanks @borisdayma
@Borda check if your comments were addressed?
| @@ -93,7 +92,7 @@ def __init__( | |||
| save_dir: Optional[str] = None, | |||
| offline: Optional[bool] = False, | |||
| id: Optional[str] = None, | |||
| anonymous: Optional[bool] = False, | |||
| anonymous: Optional[bool] = None, | |||
There was a problem hiding this comment.
This is actually still the same behavior but is more in line with the W&B documentation so less prone to potential bugs.
What does this PR do?
Fixes #6788
This PR lets users define any
wandb.init()parameter, includingresumewhich was previously forced toallow.In addition, it handles the case where arguments are passed as
kwargsvs as direct arguments.Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 馃檭