Allow importing keras_nlp without tensorflow #1660
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattdangerw
changed the title
mattdangerw
force-pushed
the
tf-import-better
branch
from
9c60479 to
692f1d6
Compare
mattdangerw
force-pushed
the
tf-import-better
branch
3 times, most recently
from
3cbc6f0 to
1a239bf
Compare
This change **does not** break the dependency of tensorflow for all text preprocessing, so most real world workflows will still require a tensorflow install. But this would allow running some modeling in Jax with all preprocessing done with a separately library. And most important, this will give friendly, actionable install instructions the first time something is used that requires a tensorflow install.
mattdangerw
force-pushed
the
tf-import-better
branch
from
1a239bf to
d1ccc73
Compare
mattdangerw
force-pushed
the
tf-import-better
branch
from
4f3ece6 to
9178ff4
Compare
mattdangerw
force-pushed
the
tf-import-better
branch
from
3170b02 to
ce8d8e6
Compare
| import keras_nlp | ||
|
|
||
|
|
||
| class NoTensorflow(unittest.TestCase): |
There was a problem hiding this comment.
Is this testing basic functions when TF is not installed? I'm curious to know how you make sure that TF is not installed for this test.
There was a problem hiding this comment.
It's in the actions.yml controlling github actions. When testing on jax and torch, as a last step we uninstall Tensorflow and run these tests.
This will give us some coverage is someone imports Tensorflow unconditionally. It should break this test.
There was a problem hiding this comment.
Got it! Thanks for explaining!
SamanehSaadat
approved these changes
Jul 10, 2024
| import keras_nlp | ||
|
|
||
|
|
||
| class NoTensorflow(unittest.TestCase): |
There was a problem hiding this comment.
Got it! Thanks for explaining!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change does not break the dependency of tensorflow for all text
preprocessing, so most real world workflows will still require a
tensorflow install.
But this would allow running some modeling in Jax with all preprocessing
done with a separately library.
And most important, this will give friendly, actionable install
instructions the first time something is used that requires a
tensorflow install.