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

Fix bug + Add Tests + Enhance docstrings (shape_equal) #751

Merged
merged 21 commits into from
Aug 20, 2023
Merged

Fix bug + Add Tests + Enhance docstrings (shape_equal) #751

merged 21 commits into from
Aug 20, 2023

Conversation

Faisal-Alsrheed
Copy link
Contributor

@Faisal-Alsrheed Faisal-Alsrheed commented Aug 19, 2023

Bug Fix:

In shape_equal function, using axis=1 resulted in the error: TypeError: 'int' object is not iterable.
but axis=[1] worked correctly.

After hours of trying :) the issue has been resolved, and now an integer, list, or tuple of integers work as expected and documented.

Improved Docstrings:
The documentation for the shape_equal function has been enhanced. It now provides a more in-depth description and includes illustrative examples.

Additional Tests:

A new set of tests has been added to ensure the proper functionality of the shape_equal function.
Why do we need tests for the shape_equal function? Because there are 7 other functions depend on it:

  1. Append ( )
  2. Average ( )
  3. Concatenate ( )
  4. Hstack ( )
  5. Stack ( )
  6. Tensordot ( )
  7. Vstack ()

Please share your feedback, thank you for the guidance, I am learning so much :)

@Faisal-Alsrheed Faisal-Alsrheed changed the title Enhance shape_equal Functionality and Tests Bug Fix and Enhance shape_equal Functionality and Tests Aug 19, 2023
@Faisal-Alsrheed Faisal-Alsrheed changed the title Bug Fix and Enhance shape_equal Functionality and Tests Bug Fix and Enhance shape_equal Docstrings and Tests Aug 19, 2023
@Faisal-Alsrheed Faisal-Alsrheed changed the title Bug Fix and Enhance shape_equal Docstrings and Tests Bug Fix + Add Tests + Enhance docstrings for shape_equal Aug 19, 2023
@Faisal-Alsrheed Faisal-Alsrheed changed the title Bug Fix + Add Tests + Enhance docstrings for shape_equal Enhance docstrings + Add Tests for shape_equal Aug 19, 2023
@Faisal-Alsrheed Faisal-Alsrheed changed the title Enhance docstrings + Add Tests for shape_equal Add Tests and Enhance shape_equal docstrings Aug 19, 2023
@Faisal-Alsrheed Faisal-Alsrheed marked this pull request as ready for review August 19, 2023 08:36
@Faisal-Alsrheed Faisal-Alsrheed changed the title Add Tests and Enhance shape_equal docstrings Fix bug + Add Tests + Enhance docstrings (shape_equal) Aug 19, 2023
Copy link
Contributor

@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.

Thanks for the PR!

keras_core/ops/numpy.py Outdated Show resolved Hide resolved
keras_core/ops/numpy.py Outdated Show resolved Hide resolved
keras_core/ops/numpy.py Outdated Show resolved Hide resolved
keras_core/ops/numpy.py Outdated Show resolved Hide resolved
@Faisal-Alsrheed
Copy link
Contributor Author

Hi @fchollet :)

Which Python function docstring standard does the Keras team use?

Please recommend a python library for docstring. Black is not working with docstring as far as u know.

I can not find any information
https://www.perplexity.ai/search/054000c8-6e38-427d-bed7-930814e4f4ed?s=u

This ?
https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings

This ?
https://github.com/keras-team/keras-cv/blob/5373b916d15544a6763347575d440570ec617495/.github/CONTRIBUTING.md#formatting-the-code

Thank you

keras_core/ops/numpy.py Outdated Show resolved Hide resolved
@fchollet
Copy link
Contributor

Black is not working with docstring as far as u know.

You can copy/paste code snippets into a black formatting tool, there are webpages that can do it.

Copy link
Contributor

@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, thank you!

@fchollet fchollet merged commit db4f8ae into keras-team:main Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants