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

Avoid import tensorflow directly #18655

Merged
merged 2 commits into from
Oct 21, 2023
Merged

Avoid import tensorflow directly #18655

merged 2 commits into from
Oct 21, 2023

Conversation

haifeng-jin
Copy link
Contributor

People may use other backends without installing TF.

from keras.api_export import keras_export
from keras.backend import KerasTensor
from keras.backend import distribution_lib
from keras.backend.common import global_state
from keras.utils.module_utils import tensorflow as tf
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the fix. Can we move this to the place that tf is used (eg the distribute_dataset method.)

@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (2683105) 78.49% compared to head (8a9a94c) 69.59%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #18655      +/-   ##
==========================================
- Coverage   78.49%   69.59%   -8.91%     
==========================================
  Files         334      334              
  Lines       32603    32605       +2     
  Branches     6377     6377              
==========================================
- Hits        25593    22690    -2903     
- Misses       5454     8452    +2998     
+ Partials     1556     1463      -93     
Flag Coverage Δ
keras 69.54% <0.00%> (-8.85%) ⬇️
keras-jax ?
keras-numpy 57.93% <0.00%> (-0.01%) ⬇️
keras-tensorflow ?
keras-torch 65.37% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
keras/distribution/distribution_lib.py 76.38% <0.00%> (-17.59%) ⬇️

... and 66 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fchollet
Copy link
Collaborator

Why was this not caught by our dedicated integration test?

@haifeng-jin
Copy link
Contributor Author

haifeng-jin commented Oct 20, 2023

Why was this not caught by our dedicated integration test?

I just found it is caught by the integration test. However, the CI did not fail even with the error presented in the log.

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Oct 21, 2023
@fchollet fchollet merged commit 0185654 into keras-team:master Oct 21, 2023
@google-ml-butler google-ml-butler bot removed awaiting review ready to pull Ready to be merged into the codebase kokoro:force-run labels Oct 21, 2023
@qlzh727
Copy link
Member

qlzh727 commented Oct 23, 2023

Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

5 participants