Skip to content

Improve error messages for see also parsing #306

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

Merged
merged 4 commits into from
Dec 9, 2020

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Dec 9, 2020

I ran into some exceptions raised while parsing improperly-formatted See Also entries and found the error messages difficult to understand. Currently, numpydoc raises a custom ParseError exception that includes the entire docstring in the error message. Also, due to the way these errors from an extension are being handled by sphinx, the type of the exception is lost when reported. An example of one of these error messages is below:

Extension error:
Handler <function mangle_signature at 0x7f559b202670> for event 'autodoc-process-signature' threw an exception (exception: floyd_warshall() is not a item name in 'Compute shortest paths between all nodes.\n\nParameters\n----------\nG : NetworkX graph\n\ncutoff : integer, optional\n    Depth at which to stop the search. Only paths of length at most\n    `cutoff` are returned.\n\nReturns\n-------\nlengths : dictionary\n    Dictionary, keyed by source and target, of shortest paths.\n\nExamples\n--------\n>>> G = nx.path_graph(5)\n>>> path = dict(nx.all_pairs_shortest_path(G))\n>>> print(path[0][4])\n[0, 1, 2, 3, 4]\n\nSee Also\n--------\nfloyd_warshall()')
make: *** [Makefile:48: html] Error 2

This PR makes two changes to improve these error messages:

  1. Switch from the custom ParseError exception, which includes the entire docstring in the message, to the internal _error_location method, which includes only the object name and file location (if _obj exists).
  2. Minor updates to _error_location to improve readability:
    • Only include _obj.__name__, which eliminates the printing of addresses
    • Don't print "in None" when the originating file isn't found.

The above exception message now looks like:

Extension error:
Handler <function mangle_signature at 0x7f54a476b310> for event 'autodoc-process-signature' threw an exception (exception: Error parsing See Also entry 'floyd_warshall()' in the docstring of all_pairs_shortest_path in repos/networkx/networkx/algorithms/shortest_paths/unweighted.py.)
make: *** [Makefile:48: html] Error 2

@rossbar
Copy link
Contributor Author

rossbar commented Dec 9, 2020

The TravisCI failure is a Python 3.5 job due to the fact that I used f-strings. I'm not sure how far back numpydoc supports, but if it's an issue I'm happy to replace the f-strings with .format - LMK!

Copy link
Collaborator

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Looks good to me! Let's just drop 3.5 support instead, can you remove it in this PR and we can merge?

@rossbar
Copy link
Contributor Author

rossbar commented Dec 9, 2020

Try close/reopen to see if that will retrigger travis with the new config

@rossbar rossbar closed this Dec 9, 2020
@rossbar rossbar reopened this Dec 9, 2020
@rossbar
Copy link
Contributor Author

rossbar commented Dec 9, 2020

Okay - I left the testing of sphinx==1.6.5 in place, but that job from 3.5 -> 3.6.

While on the topic of CI: the main numpy repo is moving away from Travis after their change in policy for OSS projects. Though the CI burden is much lower for numpydoc, I assume we should do the same here. Any thoughts/objections? I'm happy to put something in place either via github actions or adding a testing job to the existing circleCI workflow.

@larsoner
Copy link
Collaborator

larsoner commented Dec 9, 2020

+1 for moving, +1 for Azure or GitHub Actions, -1 on CircleCI (might as well distribute the load)

@larsoner larsoner merged commit bb96d0f into numpy:master Dec 9, 2020
@larsoner
Copy link
Collaborator

larsoner commented Dec 9, 2020

Thanks @rossbar !

Feel free to open a PR to work on CIs if you're up for it

@rossbar rossbar deleted the update-seealso-parse-error branch December 9, 2020 19:48
@jarrodmillman jarrodmillman added this to the 1.2.0 milestone Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants