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

update dataset_ops.py #38579

Merged
merged 4 commits into from May 6, 2020
Merged

Conversation

NeerajBhadani
Copy link
Contributor

Code consistency

Code consistency
@google-ml-butler google-ml-butler bot added the size:XS CL Change Size: Extra Small label Apr 15, 2020
@gbaned gbaned self-assigned this Apr 16, 2020
@gbaned gbaned added the comp:data tf.data related issues label Apr 16, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Apr 16, 2020
@gbaned gbaned requested a review from aaudiber April 16, 2020 03:41
aaudiber
aaudiber previously approved these changes Apr 16, 2020
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Apr 16, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 16, 2020
@gbaned gbaned added the kokoro:force-run Tests on submitted change label Apr 16, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 16, 2020
@gbaned
Copy link
Contributor

gbaned commented Apr 16, 2020

@NeerajBhadani Can you please check build failures. Thanks!

@gbaned gbaned removed the ready to pull PR ready for merge process label Apr 16, 2020
@aaudiber
Copy link
Contributor

@NeerajBhadani It appears that the line is now too long, can you shorten the example or wrap the line?

FAIL: Found 1 non-whitelisted pylint errors:
tensorflow/python/data/ops/dataset_ops.py:1659: [C0301(line-too-long), ] Line too long (87/80)

@gbaned gbaned added the kokoro:force-run Tests on submitted change label Apr 17, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 17, 2020
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Apr 20, 2020
@NeerajBhadani
Copy link
Contributor Author

@NeerajBhadani It appears that the line is now too long, can you shorten the example or wrap the line?

FAIL: Found 1 non-whitelisted pylint errors:
tensorflow/python/data/ops/dataset_ops.py:1659: [C0301(line-too-long), ] Line too long (87/80)

Thanks @aaudiber I have made the necessary changes. Let me know if anything else is required from my side.

Regards,
Neeraj

@gbaned gbaned requested a review from aaudiber April 20, 2020 11:40
aaudiber
aaudiber previously approved these changes Apr 20, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 20, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 20, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 20, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Apr 20, 2020
@NeerajBhadani
Copy link
Contributor Author

@aaudiber I have added the paratheses in order to avoid the syntax error. However, I have checked the code locally and it was working without parentheses as well.

@gbaned gbaned requested a review from aaudiber April 22, 2020 14:37
@gbaned gbaned added the awaiting review Pull request awaiting review label Apr 22, 2020
aaudiber
aaudiber previously approved these changes Apr 22, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 22, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 22, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 22, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed awaiting review Pull request awaiting review ready to pull PR ready for merge process labels Apr 23, 2020
@NeerajBhadani
Copy link
Contributor Author

@aaudiber I have checked the code locally and it seems to be working fine but here some of the test cases are failing with multiline code. Could you please help here.

@aaudiber
Copy link
Contributor

@NeerajBhadani sorry I didn't notice this at first - multi-line doctest statements require "..." on the continuation line, see https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/data/ops/dataset_ops.py#L612-L614 for example

@gbaned gbaned added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Apr 29, 2020
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Apr 29, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Apr 29, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Apr 29, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Apr 29, 2020
@tensorflowbutler tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Apr 29, 2020
@gbaned gbaned added cla: no and removed cla: yes labels Apr 30, 2020
@tensorflow-copybara tensorflow-copybara merged commit 638ba01 into tensorflow:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:data tf.data related issues ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Approved by Reviewer
Development

Successfully merging this pull request may close these issues.

None yet

7 participants