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

added processing_census example notebook #591

Merged
merged 11 commits into from Nov 7, 2019

Conversation

@vlasenkoalexey
Copy link
Contributor

vlasenkoalexey commented Oct 25, 2019

@sshrdp FYI

@review-notebook-app

This comment has been minimized.

Copy link

review-notebook-app bot commented Oct 25, 2019

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

@sshrdp

This comment has been minimized.

Copy link
Member

sshrdp commented Oct 25, 2019

Looks good!

@yongtang yongtang assigned lamberta and MarkDaoust and unassigned lamberta and MarkDaoust Oct 25, 2019
@yongtang yongtang requested review from lamberta and MarkDaoust Oct 25, 2019
@yongtang

This comment has been minimized.

Copy link
Member

yongtang commented Oct 25, 2019

@MarkDaoust @lamberta I think it would make sense to have a link to pointing to this notebook from tensorflow.org/io. Do you know which file in docs needs to be updated?

Copy link
Contributor

MarkDaoust left a comment

Hi @vlasenkoalexey,

We just got TensorFlow I/O setup on tensorflow.org.

The tensorflow.org pipeline is setup to handle notebooks. We can add a link to this, by adding the following to the docs/tutorials/_toc.yaml

  - heading: "Tutorials"
  - title: "Bigquery"
    path: https://github.com/tensorflow/io/blob/master/tensorflow_io/bigquery/processing_census_example.ipynb
    status: external

But, would it be better if we stuck this in docs/tutorials directory, then we could publish it directly to the site?

If you do move it, please rename to "bigquery.ipynb" or something like that. And add the following entries to the _toc.yaml:

  - heading: "Tutorials"
  - title: "Bigquery"
    path: /io/tutorials/bigquery

If you do the above, this will also be published as tensorflow.org/io/tutorials/bigquery

Copy link
Contributor Author

vlasenkoalexey left a comment

Moved to docs/tutorial, and used format similar to TF template.

@vlasenkoalexey vlasenkoalexey requested a review from MarkDaoust Oct 26, 2019
@vlasenkoalexey

This comment has been minimized.

Copy link
Contributor Author

vlasenkoalexey commented Nov 1, 2019

@MarkDaoust, applied changes, could you take another look

@yongtang

This comment has been minimized.

Copy link
Member

yongtang commented Nov 4, 2019

@vlasenkoalexey Looks like there is a merge conflict, can you try to rebase?

Copy link
Contributor

MarkDaoust left a comment

I resolved that conflict.

But it looks like we don't have a good way to execute this to generate the site because of the required auth.

@vlasenkoalexey would it be possible to re-save the notebook with the output included so people can see the expected result?

@vlasenkoalexey

This comment has been minimized.

Copy link
Contributor Author

vlasenkoalexey commented Nov 5, 2019

@MarkDaoust, resaved notebook with outputs

@vlasenkoalexey vlasenkoalexey requested a review from MarkDaoust Nov 6, 2019
@yongtang

This comment has been minimized.

Copy link
Member

yongtang commented Nov 6, 2019

@MarkDaoust @lamberta to take a look to see if all the issues have been resolved.

@lamberta

This comment has been minimized.

Copy link
Member

lamberta left a comment

A few comments. Looks good!

docs/tutorials/bigquery.ipynb Outdated Show resolved Hide resolved
docs/tutorials/bigquery.ipynb Show resolved Hide resolved
docs/tutorials/bigquery.ipynb Show resolved Hide resolved
docs/tutorials/bigquery.ipynb Outdated Show resolved Hide resolved
@vlasenkoalexey vlasenkoalexey requested a review from lamberta Nov 7, 2019
Copy link
Member

lamberta left a comment

Thanks

@yongtang yongtang merged commit c60dceb into tensorflow:master Nov 7, 2019
7 checks passed
7 checks passed
Ubuntu GPU Python2 Internal CI build successful
Details
Ubuntu GPU Python3 Internal CI build successful
Details
Ubuntu Python2 Internal CI build successful
Details
Ubuntu Python3 Internal CI build successful
Details
Ubuntu Sanity Check Internal CI build successful
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.