-
Notifications
You must be signed in to change notification settings - Fork 650
Making keras-contrib compatible with tf.keras #387
Changes from 64 commits
07c17fe
f80d8e0
a70847d
97219b5
746f9a2
19b0734
a09fabc
67924a3
49619b0
c251865
9724487
752f1ef
7025a6d
cc68fa6
564af63
33058bb
ee99fa9
3d167a3
dbe92e7
ae27d1b
79bb6d8
98b507d
4ca208a
9b6b0dd
82aa1dc
cbbf06d
9e380e8
a0f86a5
8a6b5cf
543a744
a10f4f0
7f28ce0
612c2a3
8a3ffe3
33b8150
abafa3a
9dbe097
9c623f3
cb6131f
516cc14
7427849
114be91
ac931af
fc2dd4d
4ec19d3
edcc433
0cf37de
79a3490
75db9e9
f4203ae
e04ab3f
99fb7ba
a2152c5
f8058c1
bbe1641
d1a9aaf
2fa39d3
4e33f67
4064e11
0e598b0
8eec1f8
b262f9f
9d02f74
a15a01b
95310a2
dd1e3c6
8757f67
701f5eb
c7505ad
b7c9769
9fdc858
5e37a62
7d662fa
45318fa
b3366c7
3d7fb42
775020c
e637aab
b959b8c
4472d24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,44 @@ We love pull requests. Here's a quick guide: | |
|
||
11. Submit your PR. If your changes have been approved in a previous discussion, and if you have complete (and passing) unit tests, your PR is likely to be merged promptly. Otherwise, well... | ||
|
||
## About keras-team/keras and tensorflow.keras | ||
|
||
This repo supports both keras-team/keras and tensorflow.keras. The way this is done is by changing all the imports in the code by parsing it. This is checked with travis.ci every time you push a commit in a pull request. | ||
|
||
There are a number of reasons why your code would work with keras-team/keras but not with tf.keras. The most common is that you use keras' private API. Since both keras are only similar in behavior with respect to their public API, you should only use this. Otherwise it's likely that the function you are using is not in the same place in tf.keras. | ||
|
||
Another gotcha is that when creating custome layers and implementing the `build` function, keras-team/keras expects as `input_shape` a tuple of ints. With tf.keras, `input_shape` is a tuple with `Dimensions` objects. This is likely to make the code incompatible. To solve this | ||
problem, you should do: | ||
|
||
```python | ||
from keras.layers import Layer | ||
from keras_contrib.utils.test_utils import to_tuple | ||
|
||
|
||
class MyLayer(Layer): | ||
... | ||
|
||
def build(self, input_shape): | ||
input_shape = to_tuple(input_shape) | ||
# now `input_shape` is a tuple of ints or None like in keras-team/keras | ||
... | ||
``` | ||
|
||
To change all the imports in your code to tf.keras to test compatibility, you can do: | ||
``` | ||
python convert_to_tf_keras.py | ||
``` | ||
|
||
To convert your codebase back to keras-team/keras, do: | ||
``` | ||
python convert_to_tf_keras.py --revert | ||
``` | ||
|
||
Not that you are strongly encouraged to commit your code before in case the parsing would go wrong. To discard all the changes you made since the previous commit: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: Note that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure we want to encourage users to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, the method I describe here is very brittle. This is because my knowledge of GIT is quite limited. Do you have a workflow in mind which would be cleaner and more robust? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would encourage to do a soft reset with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the tip! |
||
``` | ||
git reset --hard | ||
``` | ||
|
||
## A Note for Contributors | ||
|
||
Both Keras-Contrib and Keras operate under the [MIT License](LICENSE). At the discretion of the maintainers of both repositories, code may be moved from Keras-Contrib to Keras and vice versa. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import os | ||
import sys | ||
|
||
list_conversions = [('import keras.', 'import tensorflow.keras.'), | ||
gabrieldemarmiesse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
('import keras ', 'from tensorflow import keras '), | ||
('import keras\n', 'from tensorflow import keras\n'), | ||
('from keras.', 'from tensorflow.keras.'), | ||
('from keras ', 'from tensorflow.keras ')] | ||
|
||
|
||
def replace_imports(file_path, revert): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be more prudent to add a test, to make sure the created file is as expected (like we do with generated docs) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already run all the tests with the generated files. Do you have a specific test in mind? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ho, I see what you mean. I'm not in favor of including tests at running time (except for catching user errors) because this will complexify the codebase. Separating the tests and the implementation allow for a better readability. And when tests fail, we know exactly which use case caused it. It's more extensible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RaphaelMeudec do you have any simple tests in mind? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gabrieldemarmiesse What I had in mind is :
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a test in the same file (we don't have a choice, we can't import the function directly). And added the command to run it in travis. |
||
if not file_path.endswith('.py'): | ||
return False | ||
if os.path.abspath(file_path) == os.path.abspath(__file__): | ||
return False | ||
with open(file_path, 'r') as f: | ||
text = f.read() | ||
|
||
if revert: | ||
list_imports_to_change = [x[::-1] for x in list_conversions] | ||
else: | ||
list_imports_to_change = list_conversions | ||
|
||
text_updated = text | ||
for old_str, new_str in list_imports_to_change: | ||
text_updated = text_updated.replace(old_str, new_str) | ||
|
||
with open(file_path, 'w+') as f: | ||
f.write(text_updated) | ||
|
||
return text_updated != text | ||
|
||
|
||
def convert_codebase(revert): | ||
nb_of_files_changed = 0 | ||
keras_dir = os.path.dirname(os.path.abspath(__file__)) | ||
for root, dirs, files in os.walk(keras_dir): | ||
for name in files: | ||
if replace_imports(os.path.join(root, name), revert): | ||
nb_of_files_changed += 1 | ||
print('Changed imports in ' + str(nb_of_files_changed) + ' files.') | ||
print('Those files were found in the directory ' + keras_dir) | ||
|
||
|
||
def convert_to_tf_keras(): | ||
"""Convert the codebase to tf.keras""" | ||
convert_codebase(False) | ||
|
||
|
||
def convert_to_keras_team_keras(): | ||
"""Convert the codebase from tf.keras to keras-team/keras""" | ||
convert_codebase(True) | ||
|
||
|
||
if __name__ == '__main__': | ||
if '--revert' in sys.argv: | ||
convert_to_keras_team_keras() | ||
else: | ||
convert_to_tf_keras() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,23 @@ | ||
from setuptools import setup | ||
from setuptools import find_packages | ||
import os | ||
|
||
|
||
setup(name='keras_contrib', | ||
if os.environ.get('USE_TF_KERAS', None) == '1': | ||
name = 'tf_keras_contrib' | ||
install_requires = [] | ||
else: | ||
name = 'keras_contrib' | ||
install_requires = ['keras'] | ||
|
||
setup(name=name, | ||
version='2.0.8', | ||
description='Keras Deep Learning for Python, Community Contributions', | ||
author='Fariz Rahman', | ||
author_email='[email protected]', | ||
url='https://github.com/farizrahman4u/keras-contrib', | ||
license='MIT', | ||
install_requires=['keras'], | ||
install_requires=install_requires, | ||
extras_require={ | ||
'h5py': ['h5py'], | ||
'visualize': ['pydot>=1.2.0'], | ||
|
Uh oh!
There was an error while loading. Please reload this page.