Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add from_dict/from_yaml utility functions #31

Merged
merged 7 commits into from Nov 5, 2020
Merged

feat: add from_dict/from_yaml utility functions #31

merged 7 commits into from Nov 5, 2020

Conversation

hbredin
Copy link
Collaborator

@hbredin hbredin commented Nov 2, 2020

Closes #25

@hbredin
Copy link
Collaborator Author

hbredin commented Nov 2, 2020

CI fails because it looks like test_requires requirements are not installed when they should be.

@iver56 iver56 self-requested a review November 2, 2020 21:42
@iver56
Copy link
Collaborator

iver56 commented Nov 3, 2020

Yeah, sorry, test_requires isn't actually used in the CI pipeline right now. See .github/workflows/test_requirements.txt. We should probably fix this inconsistency later.

iver56
iver56 reviewed Nov 3, 2020
View reviewed changes
torch_audiomentations/utils/config.py Outdated Show resolved Hide resolved
tests/test_config.py Outdated Show resolved Hide resolved
@hbredin
fix: fix CI
d7c7e87
@hbredin
chore: streamline from_config API
fdb2482 
Bonus: this new API allows nested composition.
@hbredin
Copy link
Collaborator Author

hbredin commented Nov 4, 2020

I switched to the {'transform': ..., 'params': ...} API even for basic (i.e. not composed) transform.
Bonus: this allows nested composition.

@hbredin hbredin requested a review from iver56 November 5, 2020 13:42
@iver56 iver56 merged commit 771d9d9 into asteroid-team:master Nov 5, 2020
@iver56
Copy link
Collaborator

iver56 commented Nov 5, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpariente mpariente mpariente left review comments

@iver56 iver56 iver56 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configuration file
3 participants
@hbredin @iver56 @mpariente