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

Keeping original signatures for functions/methods #1882

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 6 additions & 50 deletions README.rst
Original file line number Diff line number Diff line change
@@ -1,28 +1,15 @@
=================
README for Sphinx
=================

Copy link
Contributor

Choose a reason for hiding this comment

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

Your changes to the main Sphinx README make it hard to merge this PR as-is.

This is the Sphinx documentation generator, see http://sphinx-doc.org/.
This is the fork of Sphinx documentation generator, see http://sphinx-doc.org/.
In this version implemented proposal from https://github.com/sphinx-doc/issues/759.



Installing
==========

Install from PyPI to use stable version::

pip install -U sphinx

Install from PyPI to use beta version::

pip install -U --pre sphinx

Install from newest dev version in stable branch::

pip install git+https://github.com/sphinx-doc/sphinx@stable

Install from newest dev version in master branch::

pip install git+https://github.com/sphinx-doc/sphinx
pip install git+https://github.com/hypnocat/sphinx

Install from cloned source::

Expand All @@ -38,6 +25,8 @@ Reading the docs

After installing::


echo "autodoc_dumb_docstring=True" >> conf.py
cd doc
make html

Expand All @@ -46,36 +35,3 @@ Then, direct your browser to ``_build/html/index.html``.
Or read them online at <http://sphinx-doc.org/>.


Testing
=======

To run the tests with the interpreter available as ``python``, use::

make test

If you want to use a different interpreter, e.g. ``python3``, use::

PYTHON=python3 make test

Continuous testing runs on travis:

.. image:: https://travis-ci.org/sphinx-doc/sphinx.svg?branch=master
:target: https://travis-ci.org/sphinx-doc/sphinx


Contributing
============

#. Check for open issues or open a fresh issue to start a discussion around a
feature idea or a bug.
#. If you feel uncomfortable or uncertain about an issue or your changes, feel
free to email sphinx-dev@googlegroups.com.
#. Fork the repository on GitHub https://github.com/sphinx-doc/sphinx
to start making your changes to the **master** branch for next major
version, or **stable** branch for next minor version.
#. Write a test which shows that the bug was fixed or that the feature works
as expected. Use ``make test`` to run the test suite.
#. Send a pull request and bug the maintainer until it gets merged and
published. Make sure to add yourself to AUTHORS
<https://github.com/sphinx-doc/sphinx/blob/master/AUTHORS> and the change to
CHANGES <https://github.com/sphinx-doc/sphinx/blob/master/CHANGES>.
13 changes: 11 additions & 2 deletions sphinx/ext/autodoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from sphinx.util.nodes import nested_parse_with_titles
from sphinx.util.compat import Directive
from sphinx.util.inspect import getargspec, isdescriptor, safe_getmembers, \
safe_getattr, object_description, is_builtin_class_method
safe_getattr, object_description, is_builtin_class_method, dumb_signature
from sphinx.util.docstrings import prepare_docstring


Expand Down Expand Up @@ -1064,6 +1064,8 @@ def format_args(self):
args = formatargspec(*argspec)
# escape backslashes for reST
args = args.replace('\\', '\\\\')
if self.env.config.autodoc_dumb_docstring:
args = dumb_signature(self.object)
return args

def document_members(self, all_members=False):
Expand Down Expand Up @@ -1116,7 +1118,11 @@ def format_args(self):
return None
if argspec[0] and argspec[0][0] in ('cls', 'self'):
del argspec[0][0]
return formatargspec(*argspec)

if self.env.config.autodoc_dumb_docstring:
return dumb_signature(initmeth)
else:
return formatargspec(*argspec)

def format_signature(self):
if self.doc_as_attr:
Expand Down Expand Up @@ -1286,6 +1292,8 @@ def format_args(self):
args = formatargspec(*argspec)
# escape backslashes for reST
args = args.replace('\\', '\\\\')
if self.env.config.autodoc_dumb_docstring:
args = dumb_signature(self.object)
return args

def document_members(self, all_members=False):
Expand Down Expand Up @@ -1520,6 +1528,7 @@ def setup(app):
app.add_config_value('autodoc_member_order', 'alphabetic', True)
app.add_config_value('autodoc_default_flags', [], True)
app.add_config_value('autodoc_docstring_signature', True, True)
app.add_config_value('autodoc_dumb_docstring', False, True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: This is not so much a “dumb” version but, maybe, a verbatim one? Also, it doesn't apply to the docstring as a whole but to the default values, ie. the argspec/signature.

app.add_config_value('autodoc_mock_imports', [], True)
app.add_event('autodoc-process-docstring')
app.add_event('autodoc-process-signature')
Expand Down
23 changes: 23 additions & 0 deletions sphinx/util/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,26 @@ def is_builtin_class_method(obj, attr_name):
if not hasattr(builtins, safe_getattr(cls, '__name__', '')):
return False
return getattr(builtins, safe_getattr(cls, '__name__', '')) is cls


def dumb_signature(functional_object):
src = inspect.getsourcelines(functional_object)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I have general concerns about this approach: There are several shortcomings in this hand-rolled parser (see examples below), which might lead to function signatures being completely misparsed. Reimplementing a Python parser with regular expressions is a poor idea anyways; ast.parse might be a much better idea.

I'd rather tune sphinx.ext.autodoc.formatargspec to fetch names instead of values when this setting is enabled. (I'd introduce a new parameter to this function which is fed from the config and then goes and tries to extract names instead of values from the source, failing gracefully if parsing fails for whatever reason.)

Also: inspect.getsourcelines can fail with an IOError for non-source distributions (eg. .pyc).


for i, l in enumerate(src):
if 'def' in l:
Copy link
Contributor

Choose a reason for hiding this comment

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

Any decorator with def in the name will break this.

break
acc = ''
for l in src[i:]:
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be much better done through a list.join expression.

acc += l.strip()
if '):' in acc:
Copy link
Contributor

Choose a reason for hiding this comment

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

Spaces between the function signature and the terminating colon (eg. def foo(bar) :) are perfectly valid Python, but will lead to this loop never being terminated (or, at least, unexpectedly exhausting the full function body.)

break

r = re.compile(r'\((.*)\)')
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks for any kind of tuple or function call inside the signature, eg. def foo(a=(1,2)): or def bar(b=get_default_for_b()).

s = r.findall(acc)
if not s:
return '()'
s = s[0]
for d in [r'^self,', r'^self', r'^cls,', r'^cls']:
Copy link
Contributor

Choose a reason for hiding this comment

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

This introduces new behaviour where def foo(self, cls, bar) is stripped down to (bar) where, without this setting, cls would be retained.

Also, IIUC it would erroneously strip any parameter prefixed with self/cls currently, eg. def bar(selfevident_parameter):.

s = re.sub(d, '', s)
s = s.strip()
return '(' + s + ')'