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

add context to signature mangling errors #95

Merged
merged 4 commits into from
Jun 18, 2017

Conversation

amueller
Copy link
Contributor

This is slightly hacky (again) but hopefully also useful.
I ran into #67 in scikit-learn, but there was no info whatsoever where this came from. Now it gives the object name and file, so I could actually fix it. Even if #67 gets merged, there is still an error about the having yield and return which would have no context. If that is also removed then this code is not needed any more.

@amueller
Copy link
Contributor Author

As an FYI: we accidentally had duplicate sections and they should have just been fixed, but without this patch it's hard to figure out where.

try:
doc = SphinxDocString(pydoc.getdoc(obj))
except ValueError as e:
filename = obj.__code__.co_filename
Copy link
Member

Choose a reason for hiding this comment

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

That's Python 3 only, needs to be:

try:
    filename = inspect.getsourcefile(obj)
except TypeError:
    filename = None

Copy link
Member

Choose a reason for hiding this comment

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

Also, the full object name should in principle contained in name --- does it get shown by Sphinx?

@amueller
Copy link
Contributor Author

@pv thanks, I now put the fix in the right place and made this much less hacky. That allowed me to add a test.

@pv pv merged commit f65cf64 into numpy:master Jun 18, 2017
@pv
Copy link
Member

pv commented Jun 18, 2017

thanks lgtm

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.

3 participants