Enhanced Docstrings and Examples in /ops/function.py, /ops/math.py and /ops/nn.py. #736

Faisal-Alsrheed · 2023-08-16T15:28:01Z

Main Changes:

Improved docstrings and added examples for segment_sum, segment_max, and rsqrt functions in keras_core/ops/math.py.
Added docstring for network_nodes in keras_core/ops/function.py
Enhanced the docstring for multi_hot function in keras_core/ops/nn.py.

My target to add very easy to understand examples.

Faisal-Alsrheed · 2023-08-16T16:03:44Z

Thank you for your patience, I'm starting to understand GitHub slowly :)

fchollet

Thanks for the PR!

fchollet · 2023-08-16T16:49:48Z

keras_core/ops/function.py

@@ -180,7 +180,7 @@ def map_graph(inputs, outputs):

    Returns:
        A tuple `(nodes, nodes_by_depth, operations, operations_by_depth)`.
-        - nodes: list of Node instances.
+        - network_nodes:dict mapping the unique node keys to the Node instances


Add space after :

fchollet · 2023-08-16T16:50:14Z

keras_core/ops/math.py

-    >>> segment_ids = keras_core.ops.convert_to_tensor([0, 1, 0, 1, 0, 1])
-    >>> segment_sum(data, segment_ids)
-    array([9 12], shape=(2,), dtype=int32)
+    >>> data = keras_core.ops.convert_to_tensor([1,2,10,20,100,200])


What was the issue with the initial code example?

The issue was that the example does not work with the JAX backend.
`ValueError: Argument 'num_segments' must be set when using the JAX backend. Received: num_segments=None
So, I have changed it to an easier-to-understand example, and now it is working with all backends.

Sounds good -- please make sure to format the code in the black style, e.g. spaces after ,

Great, Now i am using pip install black and this tool https://black.vercel.app/

fchollet · 2023-08-16T22:21:31Z

keras_core/ops/math.py

-    >>> segment_max(data, segment_ids)
-    array([9 12], shape=(2,), dtype=int32)
-    """
+    >>> data = keras_core.ops.convert_to_tensor([1,2,10,20,100,200])


Likewise here?

Firstly, the problem was that the example didn't work with the JAX backend. It raised a ValueError: Argument 'num_segments' must be set when using the JAX backend. Received: num_segments=None.

The second issue was that the output is incorrect; it seemed as if it was copied from segment_sum.

To address these, I've modified the example to make it more understandable, and it now functions correctly with all backends."

fchollet · 2023-08-16T22:21:42Z

keras_core/ops/math.py

-    >>> rsqrt(x)
-    """
-
+   >>> data = keras_core.ops.convert_to_tensor([1.0,10.0,100.0])


Use spaces are ,

fchollet · 2023-08-16T22:21:52Z

keras_core/ops/math.py

-
+   >>> data = keras_core.ops.convert_to_tensor([1.0,10.0,100.0])
+   >>> keras_core.ops.rsqrt(data)
+   Array([1.        , 0.31622776, 0.1       ], dtype=float32)


Use array (lowercase) and trim whitespace

fchollet · 2023-08-16T22:22:05Z

keras_core/ops/nn.py

@@ -1501,6 +1501,31 @@ def compute_output_spec(self, inputs):
    ]
 )
 def multi_hot(inputs, num_tokens, axis=-1, dtype=None):
+    """
+    Encodes integer labels as multi-hot vectors.


Move this to the line above

Faisal-Alsrheed · 2023-08-16T23:45:29Z

I will be more than happy to write, enhance, and check all the docstrings. Additionally, I'll add examples that are easy to understand and compatible with all backends. Please let me know if that sounds good. @fchollet

fchollet · 2023-08-17T00:19:18Z

keras_core/ops/math.py

-    >>> segment_ids = keras_core.ops.convert_to_tensor([0, 1, 0, 1, 0, 1])
-    >>> segment_sum(data, segment_ids)
-    array([9 12], shape=(2,), dtype=int32)
+    >>> data = keras_core.ops.convert_to_tensor([1,2,10,20,100,200])


Sounds good -- please make sure to format the code in the black style, e.g. spaces after ,

keras_core/ops/math.py

Faisal-Alsrheed

Done

Faisal-Alsrheed · 2023-08-17T12:05:30Z

keras_core/ops/math.py

-    >>> segment_ids = keras_core.ops.convert_to_tensor([0, 1, 0, 1, 0, 1])
-    >>> segment_sum(data, segment_ids)
-    array([9 12], shape=(2,), dtype=int32)
+    >>> data = keras_core.ops.convert_to_tensor([1,2,10,20,100,200])


Great, Now i am using pip install black and this tool https://black.vercel.app/

keras_core/ops/math.py

Faisal-Alsrheed

Done

Faisal-Alsrheed · 2023-08-17T12:54:17Z

Great now I am using black python library and this website https://black.vercel.app/ for formatting.

Faisal-Alsrheed · 2023-08-17T12:54:32Z

My next step will be to check all the functions docstring and then discuss with you a standard way to write them even the order including examples and reference and so on

fchollet · 2023-08-17T16:34:10Z

Great now I am using black python library and this website https://black.vercel.app/ for formatting.

Once you have installed black you can just run it from the terminal, e.g. black file/path/

fchollet

Thanks for the update!

fchollet · 2023-08-17T16:34:36Z

keras_core/ops/math.py

-    >>> segment_ids = keras_core.ops.convert_to_tensor([0, 1, 0, 1, 0, 1])
-    >>> segment_max(data, segment_ids)
-    array([9 12], shape=(2,), dtype=int32)
+        >>> data = keras_core.ops.convert_to_tensor([1, 2, 10, 20, 100, 200])


Unindent: the code example should have the indent as the rest of the docstring

fchollet · 2023-08-17T16:34:46Z

keras_core/ops/math.py

@@ -688,16 +690,17 @@ def stft(
 class Rsqrt(Operation):
    """Computes reciprocal of square root of x element-wise.

-    Args:
-        x: input tensor
+     Args:


Same, fix indent

fchollet

LGTM, thank you!

Faisal-Alsrheed added 2 commits August 16, 2023 15:21

docstrings improvement

217e396

fix formatting issues

8cfecc5

Faisal-Alsrheed marked this pull request as ready for review August 16, 2023 15:55

fchollet reviewed Aug 16, 2023

View reviewed changes

format enhancement

a466f8e

Faisal-Alsrheed requested a review from fchollet August 16, 2023 23:49

fchollet reviewed Aug 17, 2023

View reviewed changes

using the Black Code Style

064593a

Faisal-Alsrheed commented Aug 17, 2023

View reviewed changes

fchollet reviewed Aug 17, 2023

View reviewed changes

fix indents

a677194

Faisal-Alsrheed requested a review from fchollet August 17, 2023 20:52

fchollet approved these changes Aug 17, 2023

View reviewed changes

fchollet merged commit 0dda60f into keras-team:main Aug 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced Docstrings and Examples in /ops/function.py, /ops/math.py and /ops/nn.py. #736

Enhanced Docstrings and Examples in /ops/function.py, /ops/math.py and /ops/nn.py. #736

Faisal-Alsrheed commented Aug 16, 2023 •

edited

Loading

Faisal-Alsrheed commented Aug 16, 2023

fchollet left a comment

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023

fchollet Aug 17, 2023

Faisal-Alsrheed Aug 17, 2023

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023 •

edited

Loading

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023

fchollet Aug 16, 2023

Faisal-Alsrheed Aug 16, 2023

Faisal-Alsrheed commented Aug 16, 2023

fchollet Aug 17, 2023

Faisal-Alsrheed left a comment •

edited

Loading

Faisal-Alsrheed Aug 17, 2023

Faisal-Alsrheed left a comment

Faisal-Alsrheed commented Aug 17, 2023

Faisal-Alsrheed commented Aug 17, 2023

fchollet commented Aug 17, 2023

fchollet left a comment

fchollet Aug 17, 2023

fchollet Aug 17, 2023

fchollet left a comment

Enhanced Docstrings and Examples in /ops/function.py, /ops/math.py and /ops/nn.py. #736

Enhanced Docstrings and Examples in /ops/function.py, /ops/math.py and /ops/nn.py. #736

Conversation

Faisal-Alsrheed commented Aug 16, 2023 • edited Loading

Faisal-Alsrheed commented Aug 16, 2023

fchollet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faisal-Alsrheed Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faisal-Alsrheed commented Aug 16, 2023

Choose a reason for hiding this comment

Faisal-Alsrheed left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faisal-Alsrheed left a comment

Choose a reason for hiding this comment

Faisal-Alsrheed commented Aug 17, 2023

Faisal-Alsrheed commented Aug 17, 2023

fchollet commented Aug 17, 2023

fchollet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fchollet left a comment

Choose a reason for hiding this comment

Faisal-Alsrheed commented Aug 16, 2023 •

edited

Loading

Faisal-Alsrheed Aug 16, 2023 •

edited

Loading

Faisal-Alsrheed left a comment •

edited

Loading