Skip to content

[WIP] Add failing test case for image dataset transforms #244

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 21 commits into from

Conversation

kyleabeauchamp
Copy link
Contributor

Initial attempt at getting test coverage for #243

@kyleabeauchamp kyleabeauchamp requested a review from a team as a code owner July 4, 2019 16:34
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Jul 4, 2019
@kyleabeauchamp
Copy link
Contributor Author

OK, so I've rebased this PR off of #330. I can confirm that the failing test case is now passing.

@kyleabeauchamp
Copy link
Contributor Author

For sanity, we probably don't want to review this until #330 is merged.

@facaiy
Copy link
Member

facaiy commented Jul 5, 2019

Looks good, Kyle. As we add test case for unknown shape tensor, is it necessary to retain
tf.data.Dataset test case? What do you think?

@kyleabeauchamp
Copy link
Contributor Author

kyleabeauchamp commented Jul 5, 2019 via email

@facaiy
Copy link
Member

facaiy commented Jul 5, 2019

Make sense to me as well, but I prefer to make unit test simple according to Occam's Razor. How about letting us add test case with tf.data.Dataset later when we think it necessary?

@kyleabeauchamp
Copy link
Contributor Author

kyleabeauchamp commented Jul 6, 2019 via email

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants