Skip to content

self included in list of params for method #220

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

Closed
thequackdaddy opened this issue May 27, 2019 · 17 comments
Closed

self included in list of params for method #220

thequackdaddy opened this issue May 27, 2019 · 17 comments

Comments

@thequackdaddy
Copy link
Contributor

Hello

I'm trying to fix some issues with statsmodels documentation.

One issue that we've uncovered is that numpydoc 0.9.0 (and 0.9.1) includes self in the list of params for a method, when it would be preferable if it didn't. That's different than what 0.8.0 did.

Screenshot of 0.9.0...

Screen Shot 2019-05-27 at 12 19 05 PM

Screenshot of 0.8.0...

Screen Shot 2019-05-27 at 12 23 40 PM

Any ideas how to fix?

@jnothman
Copy link
Member

jnothman commented May 27, 2019 via email

@pv
Copy link
Member

pv commented May 27, 2019

Is it numpydoc, or sphinx version upgrade? The table is probably constructed by sphinx.ext.autosummary, signature mangling is maybe from numpydoc but I don't recall how this works exactly any more...

@thequackdaddy
Copy link
Contributor Author

Its just a numpydoc upgrade. I am using sphinx 2.0.1 on both.

From what I I can figure, you can do...

from numpydoc.docscrape_sphinx import get_doc_object
import statsmodels.api as sm

get_doc_objecdt(sm.GLM)

and you get the same output for both. That's just an autosummray table with just the method names (no signature/parameters).

So that looks right. But the signature is different. I can't really tell how the signature is generated from the code.

@thequackdaddy
Copy link
Contributor Author

Git bisect has lead me to think the issue is here:

2cdd4d9

@jnothman
Copy link
Member

jnothman commented May 28, 2019 via email

@thequackdaddy
Copy link
Contributor Author

I'm pretty certain that is it. I changed that code back in a development version and it works.

I can't seem to figure out how the code works, however...

In [1]: import statsmodels.api as sm

In [2]: from numpydoc.docscrape_sphinx import SphinxDocString, get_doc_object

In [3]: import pydoc

In [4]: SphinxDocString(pydoc.getdoc(sm.GLM.fit))
Out[4]: <numpydoc.docscrape_sphinx.SphinxDocString at 0x1204bf630>

In [5]: SphinxDocString(pydoc.getdoc(sm.GLM.fit))['Signature']
Out[5]: ''

In [6]: get_doc_object(sm.GLM.fit)['Signature']
Out[6]: "fit(self, start_params=None, maxiter=100, method='IRLS', tol=1e-08, scale=None, cov_type='nonrobust', cov_kwds=None, use_t=None, full_output=True, disp=False, max_start_irls=3, \\*\\*kwargs)"

I don't know why running this way, the "old" way doesn't work. It must be adding the signature before it goes through this. I'm not sure how the app.connect('autodoc-process-signature', mangle_signature) line works...

@thequackdaddy
Copy link
Contributor Author

(my guess is that autodoc is parsing it, and realizing that this is method and dropping the first parameter, then prepending the signature on the docs. Not sure how to setup a test of that, however.)

@thequackdaddy
Copy link
Contributor Author

Ok, better guess. I think before, this was broken and didn't return signatures when they weren't explicitly stated in the docstring. Then I'm guessing sphinx added the correct one.

With the "fix" (that reverts behavior) doc['Signature'] is ''.

loading intersphinx inventory from https://docs.scipy.org/doc/numpy/objects.inv...
loading intersphinx inventory from https://docs.python.org/3/objects.inv...
loading intersphinx inventory from https://matthew-brett.github.io/pydagogue/objects.inv...
loading intersphinx inventory from https://patsy.readthedocs.io/en/latest/objects.inv...
loading intersphinx inventory from https://pandas.pydata.org/pandas-docs/stable/objects.inv...
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 90 source files that are out of date
updating environment: 376 added, 0 changed, 0 removed
> /Users/peter/git/numpydoc/numpydoc/numpydoc.py(222)mangle_signature()ar_model.GLM
-> sig = doc['Signature'] or getattr(obj, '__text_signature__', None)
(Pdb) doc
<numpydoc.docscrape_sphinx.SphinxDocString object at 0x12483ae48>
(Pdb) doc['Signature']
''
(Pdb) c
> /Users/peter/git/numpydoc/numpydoc/numpydoc.py(222)mangle_signature()ar_model.GLM.fit
-> sig = doc['Signature'] or getattr(obj, '__text_signature__', None)
(Pdb) doc
<numpydoc.docscrape_sphinx.SphinxDocString object at 0x126d64320>
(Pdb) doc['Signature']
''
(Pdb)

@thequackdaddy
Copy link
Contributor Author

Ok I think it boils down to this...

Before, this whole section got skipped... its not in the inheritance of SphinxDocString.

class FunctionDoc(NumpyDocString):
def __init__(self, func, role='func', doc=None, config={}):
self._f = func
self._role = role # e.g. "func" or "meth"
if doc is None:
if func is None:
raise ValueError("No function or docstring given")
doc = inspect.getdoc(func) or ''
NumpyDocString.__init__(self, doc)
if not self['Signature'] and func is not None:
func, func_name = self.get_func()
try:
try:
signature = str(inspect.signature(func))
except (AttributeError, ValueError):
# try to read signature, backward compat for older Python
if sys.version_info[0] >= 3:
argspec = inspect.getfullargspec(func)
else:
argspec = inspect.getargspec(func)
signature = inspect.formatargspec(*argspec)
signature = '%s%s' % (func_name, signature.replace('*', r'\*'))
except TypeError:
signature = '%s()' % func_name
self['Signature'] = signature

Now, this runs, and adds the signature that didn't exist before. Because the signature is added, my theory is that autodoc doesn't use its (slightly different) logic to add the signature.

https://github.com/sphinx-doc/sphinx/blob/f63abac2cad2664a8af816017f0f997bae510d14/sphinx/ext/autodoc/__init__.py#L1313-L1324

@jnothman
Copy link
Member

What a strange interaction. I'm struggling to get my head around it.

@jnothman
Copy link
Member

I don't understand how #200 worked really.

@thequackdaddy
Copy link
Contributor Author

thequackdaddy commented May 30, 2019

Let me rephrase.

It was broken before. SphinxDocString(pydoc.getdoc(class.method))['Signature'] would always return '' when the signature is not included in the actual docstring.

#200 "fixed" it.

Now get_doc_object(class.method) creates an instance of SphinxFunctionDoc(obj). That inherits FuncitonDoc and runs FunctionDoc.__init__ where the code above generates a signature. So get_doc_object(class.method)['Signature'] will not be ''. There is no logic to eliminate self from method signatures in this code.

Then--and here I haven't looked at sphinx's autodoc--since a signature already exists, autodoc just relies on the provided signature.

Before #200, since there was no signature, sphinx's autodoc used its logic to create the signature and its logic was clever enough to drop self.

@jnothman
Copy link
Member

jnothman commented May 31, 2019 via email

@thequackdaddy
Copy link
Contributor Author

so you think the solution is to adopt autodoc's logic?

I guess that would make sense. Either (hard way) duplicate their logic or (easy way) create a Signature only if it already exists in the docstring. Otherwise, just create a Signature of ''.

@thequackdaddy
Copy link
Contributor Author

thequackdaddy commented Jun 1, 2019

I updated #221 to add the functionality I'm thinking about. Let me know if you think this is on the right path.

(edit... wrong number)

@fladd
Copy link

fladd commented Oct 26, 2019

Is there a way to fix this for ourselves (besides reverting to 0.8.0)?

@thequackdaddy
Copy link
Contributor Author

Closed by #221

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

No branches or pull requests

4 participants