-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Replace meta_tags.csv with hparams.yaml #1271
Replace meta_tags.csv with hparams.yaml #1271
Conversation
Co-Authored-By: Jirka Borovec <[email protected]>
Co-Authored-By: Jirka Borovec <[email protected]>
…m/S-aiueo32/pytorch-lightning into feature/support-hierarchical-dict
Hello @S-aiueo32! Thanks for updating this PR.
Comment last updated at 2020-05-13 11:10:28 UTC |
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.
cool. maybe a test for saving to and loading from a yaml file would be good.
requirements.txt
Outdated
@@ -5,4 +5,4 @@ numpy>=1.16.4 | |||
torch>=1.1 | |||
tensorboard>=1.14 | |||
future>=0.17.1 # required for builtins in setup.py | |||
|
|||
pyyaml>=5.3.1 |
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.
is it really the minimal version we can use?
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.
it's the current latest version
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.
Could we put there like 3.x or lover since we are using quite general features?
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.
@S-aiueo32 Don't forget to also add it to the environment.yml file
This pull request is now in conflict... :( |
@S-aiueo32 how is it going? it would be great to have it in next week release 0.7.2 |
@Borda I'm sorry, but I'm too busy for about two weeks this week to be able to respond. I recognize that the major changes are complete and all that remains is testing and minor revisions. If you don't mind, could you let the core contributor take over the work? |
@Borda mind taking over this one? |
This pull request is now in conflict... :( |
changelog need to be rebased on new release #1419 |
@S-aiueo32 may you pls allow editing your PR, I have prepared to rebase, but don't have permission to push it... 🤖 |
@Borda confirmed? |
Co-authored-by: Adrian Wälchli <[email protected]>
Co-authored-by: Adrian Wälchli <[email protected]>
bc64951
to
5d27e34
Compare
@Borda @awaelchli @williamFalcon All checks pass, thanks! |
it looks good to me, I have pinged @PyTorchLightning/core-contributors to get this done |
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.
LGTM
@S-aiueo32 Great work! and thx for your patience 🦝 |
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.
shall we include a test for tests/test_deprecated.py
?
completely forgot... @S-aiueo32 mind make followup or with deprecation tests |
Before submitting
What does this PR do?
Fixes #1212 .