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 signature of functions and methods in generated docs #10743

Merged
merged 3 commits into from
Jul 22, 2018

Conversation

mkaze
Copy link
Contributor

@mkaze mkaze commented Jul 21, 2018

Summary

This PR resolves two issues:

  1. The get_function_signature in autogen.py duplicates module name for non-method functions. For example, it returns keras.preprocessing.sequence.keras.preprocessing.sequence.pad_sequences instead of keras.preprocessing.sequence.pad_sequences.

  2. Method functions and non-method functions should be distinguished from each other. Since, the module name of non-method functions should be included in the signature in the docs (like keras.preprocessing.sequence.pad_sequences(...)) whereas for the methods of classes it should not be included (like compile(...) for Sequential class). Therefore, a new 'methods' key is introduced to make this distinction.

Note that I temporary resolved the first issue by a merged PR (#10664); however that PR did not address the underlying problem.

Related Issues

#10658
#10662

PR Overview

  • [n] This PR requires new unit tests [y/n] (make sure tests are included)
  • [n] This PR requires to update the documentation [y/n] (make sure the docs are up-to-date)
  • [y] This PR is backwards compatible [y/n]
  • [n] This PR changes the current API [y/n] (all API changes need to be approved by fchollet)

Copy link
Contributor

@Dref360 Dref360 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks

@Dref360 Dref360 merged commit aab55e6 into keras-team:master Jul 22, 2018
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