|
1 |
| -# Coding guidelines and standards |
| 1 | +## Coding guidelines and standards |
2 | 2 |
|
3 |
| -## Coding Style |
| 3 | +This document is a reference guide for contributing new ops to the tensorflow-io project. |
4 | 4 |
|
5 |
| -#### C++ |
| 5 | +### Coding Style |
6 | 6 |
|
7 |
| -C++ code should conform to [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html). |
| 7 | +- C++ code should conform to [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html). |
| 8 | +- Python code should conform to [PEP8](https://www.python.org/dev/peps/pep-0008/). |
8 | 9 |
|
9 | 10 |
|
10 |
| -#### Python |
11 |
| - |
12 |
| -Python code should conform to [PEP8](https://www.python.org/dev/peps/pep-0008/). |
13 |
| - |
14 |
| -##### Running Python and Bazel Style Checks |
15 |
| - |
16 |
| -Style checks for Python and Bazel can be run with the following commands |
17 |
| -(docker has to be available): |
18 |
| - |
19 |
| -```sh |
20 |
| -$ bash -x -e .travis/lint.sh |
| 11 | +Style checks for C++, Python and Bazel can be automatically rectified with the following command: |
21 | 12 | ```
|
22 |
| - |
23 |
| -In case there are any Bazel style errors, the following command could be invoked |
24 |
| -to fix and Bazel style issues: |
25 |
| - |
26 |
| -```sh |
27 |
| -$ docker run -i -t --rm -v $PWD:/v -w /v --net=host golang:1.12 bash -x -e -c 'go get github.com/bazelbuild/buildtools/buildifier && buildifier $(find . -type f \( -name WORKSPACE -or -name BUILD -or -name *.BUILD \))' |
| 13 | +$ bazel run //tools/lint:lint |
28 | 14 | ```
|
29 | 15 |
|
30 |
| -After the command is run, any Bazel files with style issues will have been modified and corrected. |
31 |
| - |
32 |
| - |
33 |
| -## Naming Conventions |
| 16 | +### Naming Conventions |
34 | 17 |
|
35 |
| -Any new operation that is introduced in Tensorflow I/O project should be prefixed with a pre-defined text. |
36 |
| -This is done to ensure that different Tensorflow projects (for e.g Tensorflow I/O) declare their operation names that are globally unique, |
37 |
| -across the entire Tensorflow ecosystem. |
| 18 | +Any new op that is introduced in Tensorflow I/O should have a pre-defined prefix. Adherence to this guideline ensures that different Tensorflow projects (for e.g Tensorflow I/O) declare their op names that are globally unique, across the entire Tensorflow ecosystem. |
38 | 19 |
|
39 |
| -For more details on defining custom operations, please refer this [tensorflow RFC](https://github.com/tensorflow/community/pull/126). |
| 20 | +- For details on writing custom ops, please refer this [guide](https://www.tensorflow.org/guide/create_op). |
| 21 | +- For more details on naming custom ops, please refer this [tensorflow RFC](https://github.com/tensorflow/community/pull/126). |
40 | 22 |
|
41 |
| -Depending on the module's programming language (C++ or python), pre-defined prefix text may vary. This is detailed below. |
42 |
| - |
| 23 | +Depending on the module's programming language (C++ or python), the pre-defined prefix may vary. |
43 | 24 | #### C++
|
44 | 25 |
|
45 |
| -Any new operation that is introduced in C++ module should be prefixed with text “IO>”. |
46 |
| - |
47 |
| -for example: |
48 |
| - |
49 |
| -A new operation named “ReadAvro” should be registered as “IO>ReadAvro”. |
50 |
| - |
51 |
| -##### Regular way: |
52 |
| - |
53 |
| -```text |
54 |
| -REGISTER_OP("ReadAvro") |
55 |
| - ... |
56 |
| - ... |
57 |
| - ... |
58 |
| - }); |
| 26 | +The C++ based ops are placed in `tensorflow_io/core/ops` and are prefixed with text `IO>` |
| 27 | +while registering. For instance, a new op named `AudioResample` should be registered as |
| 28 | +`IO>AudioResample`. |
| 29 | + |
| 30 | +```cc |
| 31 | +// tensorflow_io/core/ops/audio_ops.cc |
| 32 | +... |
| 33 | +REGISTER_OP("IO>AudioResample") |
| 34 | + .Input("input: T") |
| 35 | + .Input("rate_in: int64") |
| 36 | + .Input("rate_out: int64") |
| 37 | + .Output("output: T") |
| 38 | + .Attr("T: type") |
| 39 | + .SetShapeFn([](shape_inference::InferenceContext* c) { |
| 40 | + c->set_output(0, c->MakeShape({c->UnknownDim(), c->UnknownDim()})); |
| 41 | + return Status::OK(); |
| 42 | + }); |
59 | 43 | ```
|
| 44 | +_Observe how the op name is prefixed with `IO>`_ |
60 | 45 |
|
61 |
| -##### Suggested way: |
62 |
| - |
63 |
| -```text |
64 |
| -REGISTER_OP("IO>ReadAvro") |
65 |
| - ... |
66 |
| - ... |
67 |
| - ... |
68 |
| - }); |
69 |
| -``` |
| 46 | +The next step would be to write the kernels for this newly defined op. The kernel implementations are placed in `tensorflow_io/core/kernels` and implement the business logic for the op. Additional details can be found in this [guide](https://www.tensorflow.org/guide/create_op). |
| 47 | +#### Python |
70 | 48 |
|
71 |
| -Please note in suggested way, how the operation name is prefixed with "IO>". |
| 49 | +Any new op that is introduced in a python module and that which corresponds to an op in C++, must be prefixed with `io_`. This ensures that the C++ op is properly bound to this python target. |
| 50 | +For example, the python equivalent declaration of above mentioned C++ op named `IO>AudioResample` would be `io_audio_resample`. |
72 | 51 |
|
73 |
| -#### Python |
| 52 | +```python |
| 53 | +from tensorflow_io.python.ops import core_ops |
74 | 54 |
|
75 |
| -Any new operation that is introduced in python module and that which corresponds to the operation in C++ module, |
76 |
| -should be prefixed with "io_". This ensures that C++ operation binding is mapped correctly. |
| 55 | +def resample(input, rate_in, rate_out, name=None): |
| 56 | + """resample audio""" |
77 | 57 |
|
78 |
| -for example: |
| 58 | + # implementation logic# |
79 | 59 |
|
80 |
| -The python equivalent declaration of above operation named “IO>ReadAvro” would be "io_read_avro". |
| 60 | + return core_ops.io_audio_resample(...) |
| 61 | +``` |
81 | 62 |
|
82 |
| -```text |
83 |
| -def read_avro(filename, schema, column, **kwargs): |
84 |
| - """read_avro""" |
85 |
| - ... |
86 |
| - ... |
87 |
| - ... |
88 |
| - return core_ops.io_read_avro(...); |
89 |
| -``` |
| 63 | +Few things to note: |
| 64 | +- The C++ op name is converted to lowercase and is `_` delimited i.e, `io_audio_resample` is now bound to `IO>AudioResample`. |
| 65 | +- The `io_audio_resample` target is available in `core_ops` This import hierarchy is due to the nature of the build targets defined [here](https://github.com/tensorflow/io/blob/master/tensorflow_io/BUILD) |
0 commit comments