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

Image Utils #13

Merged
merged 69 commits into from Jul 11, 2019
Merged

Image Utils #13

merged 69 commits into from Jul 11, 2019

Conversation

kmh4321
Copy link
Contributor

@kmh4321 kmh4321 commented Mar 14, 2019

Fixes #8.

maxfw/utils/test_image_utils.py and maxfw/utils/test_image.png are temporary files for testing purposes and can be deleted when the PR is approved.

@kmh4321 kmh4321 added the enhancement New feature or request label Mar 14, 2019
@kmh4321 kmh4321 self-assigned this Mar 14, 2019
@kmh4321 kmh4321 requested a review from djalova March 14, 2019 21:01
maxfw/utils/image_utils.py Outdated Show resolved Hide resolved
Copy link

@bdwyer2 bdwyer2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can add an __init__.py to the maxfw folder then set travis to run the test script?

maxfw/utils/test_image_utils.py Outdated Show resolved Hide resolved
bdwyer2 and others added 2 commits March 14, 2019 16:08
Co-Authored-By: kmh4321 <kmh4321@gmail.com>
@splovyt splovyt self-requested a review March 15, 2019 00:23
.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
06/20 - update to the image_utils branch
maxfw/core/utils.py Outdated Show resolved Hide resolved
@splovyt
Copy link
Contributor

splovyt commented Jun 24, 2019

I resolved most issues, and have added an initial version of the extended standardize functionality. @kmh4321 can you briefly review the standardize function and tests before pass the ball back to Nick? Thanks

@kmh4321 kmh4321 requested a review from MLnick June 26, 2019 20:32
Copy link

@MLnick MLnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good - a few comments on tests and on the redirect errors fn.

maxfw/tests/test_image_utils.py Outdated Show resolved Hide resolved
maxfw/tests/test_image_utils.py Outdated Show resolved Hide resolved
maxfw/tests/test_image_utils.py Show resolved Hide resolved
maxfw/core/utils.py Show resolved Hide resolved
maxfw/core/utils.py Outdated Show resolved Hide resolved
maxfw/tests/test_image_utils.py Outdated Show resolved Hide resolved
maxfw/utils/image_functions.py Show resolved Hide resolved
maxfw/core/utils.py Outdated Show resolved Hide resolved
maxfw/core/utils.py Outdated Show resolved Hide resolved
@kmh4321 kmh4321 requested a review from xuhdev July 8, 2019 23:16
Copy link

@MLnick MLnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Let's go ahead and merge. Before we do any maxfw release, let's do some testing on using image utils on a few models (we may uncover issues or bugs etc).

@splovyt
Copy link
Contributor

splovyt commented Jul 11, 2019

@xuhdev

Copy link
Contributor

@xuhdev xuhdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@splovyt splovyt merged commit c4f0a80 into IBM:master Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add image utils
6 participants