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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with not imported tensorflow #100

Merged
merged 2 commits into from Oct 7, 2019

Conversation

@lc0
Copy link
Contributor

commented Oct 4, 2019

Seems like in original version they used compat.v2 and after it was refactored to tf2, but somehow left broken.

As a result current live version is broken - no import of tensorflow. I removed enable_v2_behavior since colab does support tf2 with magic above.

PS: feels like I am the first one checking these notebooks 馃檲

@lc0 lc0 requested review from lamberta, MarkDaoust and wolffg as code owners Oct 4, 2019
@googlebot googlebot added the cla: yes label Oct 4, 2019
@review-notebook-app

This comment has been minimized.

Copy link

commented Oct 4, 2019

Check out this pull request on聽 ReviewNB

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

@lc0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

although, if a user runs this notebook locally we might still keep tf.enable_v2_behavior(). In this case do we have a minimum version checks to support enable_v2_behavior?

@lamberta

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

feel like I am first one checking these notebooks 馃檲

You might be!

@lamberta lamberta requested a review from adammichaelwood Oct 4, 2019
@lc0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

Once we decide with preferred to do, I would send other ones in a single PR as well. The same issue

@lc0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

@lamberta so shall I update the rest of notebooks in the same PR?

@MarkDaoust

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2019

@lamberta so shall I update the rest of notebooks in the same PR?

Yes that would be great.

Thanks.

@lc0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

just finished reviewing and updated the rest of notebooks cc @MarkDaoust @lamberta

@lc0

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2019

I would also recommend to merge these as soon as possible, since currently the notebooks are failing and they are live on udacity

TensorFlow-Examples-Copybara pushed a commit that referenced this pull request Oct 7, 2019
PiperOrigin-RevId: 273343905
@TensorFlow-Examples-Copybara TensorFlow-Examples-Copybara merged commit ec998a3 into tensorflow:master Oct 7, 2019
2 checks passed
2 checks passed
cla/google All necessary CLAs are signed
import/copybara Change imported to the internal review system
Details
@MarkDaoust

This comment has been minimized.

Copy link
Collaborator

commented Oct 7, 2019

Thanks again. Everything should be fixed now, but do let us know if you run into any other problems.

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鈥檛 perform that action at this time.