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.

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

Adding function to get augmentation transforms #889

Merged
merged 2 commits into from
Jun 29, 2024

Conversation

szmazurek
Copy link
Collaborator

Fixes #888

Proposed Changes

  • function for getting the augmentation transforms from params dict
  • using that function in dataset constructor

Checklist

  • CONTRIBUTING guide has been followed.
  • PR is based on the current GaNDLF master .
  • Non-breaking change (does not break existing functionality): provide as many details as possible for any breaking change.
  • Function/class source code documentation added/updated (ensure typing is used to provide type hints, including and not limited to using Optional if a variable has a pre-defined value).
  • Code has been blacked for style consistency and linting.
  • If applicable, version information has been updated in GANDLF/version.py.
  • If adding a git submodule, add to list of exceptions for black styling in pyproject.toml file.
  • Usage documentation has been updated, if appropriate.
  • Tests added or modified to cover the changes; if coverage is reduced, please give explanation.
  • If customized dependency installation is required (i.e., a separate pip install step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].

@szmazurek szmazurek requested a review from sarthakpati June 28, 2024 07:28
@szmazurek szmazurek self-assigned this Jun 28, 2024
@szmazurek szmazurek requested a review from a team as a code owner June 28, 2024 07:28
Copy link
Contributor

github-actions bot commented Jun 28, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@sarthakpati
Copy link
Collaborator

@szmazurek szmazurek changed the base branch from master to new-apis_v0.1.0-dev June 28, 2024 15:55
@szmazurek
Copy link
Collaborator Author

Member

Done, not sure if we should re-run the tests somehow

@szmazurek szmazurek merged commit b203928 into new-apis_v0.1.0-dev Jun 29, 2024
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 29, 2024
@sarthakpati sarthakpati deleted the feature/augmentations_getter branch June 29, 2024 14:48
@VukW VukW restored the feature/augmentations_getter branch June 29, 2024 20:28
@VukW
Copy link
Contributor

VukW commented Jun 29, 2024

Hi! Folks, by some strange reason this PR didn't trigger CI tests at all. However, it brought a bug (failing test) inside base branch, that failed in a neighbor PR: https://github.com/mlcommons/GaNDLF/actions/runs/9724955407/job/26841721449?pr=890 I'm not sure if it's the only failure. @szmazurek can you plz run the tests locally to check if everything behaves as you expected in the PR?

@szmazurek
Copy link
Collaborator Author

@VukW on it!

@sarthakpati sarthakpati deleted the feature/augmentations_getter branch June 30, 2024 02:33
@szmazurek szmazurek restored the feature/augmentations_getter branch June 30, 2024 14:15
@sarthakpati
Copy link
Collaborator

@szmazurek are you looking into the error? Starting to see this pop up for other PRs as well.

@VukW
Copy link
Contributor

VukW commented Jul 1, 2024

Checking fix here: c312e24
#890

@sarthakpati
Copy link
Collaborator

Checking fix here: c312e24 #890

Thanks, @VukW!

I am guessing that fixed it, because at least one of the tests that was breaking before is now functional.

@sarthakpati sarthakpati deleted the feature/augmentations_getter branch July 1, 2024 16:25
@VukW
Copy link
Contributor

VukW commented Jul 1, 2024

+, all tests are running successfully now. Merged the fix together with #890 .

@szmazurek
Copy link
Collaborator Author

Hey guys, I had a chance to look at that closely just now, but I see @VukW already fixed that - thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] add function to retrieve augmentations from config
3 participants