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 class and line number to warning about unknown section #101

Merged
merged 1 commit into from
Sep 17, 2017

Conversation

amueller
Copy link
Contributor

Fixes #99

@amueller
Copy link
Contributor Author

It looks like right now it raises multiple warnings for each occurrence (for sklearn) but the test only shows one warning... hm...

@jnothman
Copy link
Member

I've not checked this in the case of the current patch, but one thing to be careful with is that the object that a docstring applies to is not necessarily in the same file as where the docstring is defined. Apart from directly setting __doc__ to some variable from elsewhere (which doesn't happen in scikit-learn, but does in Pandas and, I think, Numpy), this happens often through inheritance.

Btw, @amueller, you may be interested in jnothman@1d21eb1 where I get line offsets within a docstring for parameter entries. I just haven't quite pulled this together into something useful yet :|

@amueller
Copy link
Contributor Author

amueller commented Jul 20, 2017 via email

@stefanv
Copy link
Contributor

stefanv commented Jul 20, 2017

I ran into this exact error recently: it said "error in None from None", or something like that. So, I'm all for iterative improvement.

@jnothman
Copy link
Member

jnothman commented Jul 20, 2017 via email

@rgommers
Copy link
Member

+1 helpful indeed. Doesn't work for everything but does for plain Python code at least. Just tried it on SciPy and got a couple of each of these:

/Users/rgommers/Library/Python/3.5/lib/python/site-packages/numpydoc/docscrape.py:360: 
UserWarning: Unknown section Exampes in the docstring of None in None.
  warn(msg)
/Users/rgommers/Library/Python/3.5/lib/python/site-packages/numpydoc/docscrape.py:360: 
UserWarning: Unknown section Parameers in the docstring of <function periodogram at 0x10ac9e840> in /Users/rgommers/Code/scipy/scipy/signal/spectral.py.
  warn(msg)

@rgommers
Copy link
Member

@amueller not sure if you wanted to check out that commit of @jnothman for this PR, but I'd be happy to merge this now.

@rgommers
Copy link
Member

@jnothman and @amueller looks like you are both up to speed with the numpydoc internals and have an interest in the package. Do you want commit rights?

@amueller
Copy link
Contributor Author

@rgommers Maybe I should remove the "of None in None"? But I guess that's details.
Happy to have this merged now, or I can detect "None".
And I'd take commit rights, but I can't promise I'll review anything. I'm a couple hundred reviews behind on sklearn ;)

@stefanv
Copy link
Contributor

stefanv commented Jul 21, 2017

Yes, please spread the commit rights to numpydoc! Thanks for taking initiative, Ralph.

@rgommers
Copy link
Member

rgommers commented Jul 24, 2017

Maybe I should remove the "of None in None"? But I guess that's details.

Yeah that doesn't bother me, removing it won't locating the offending line easier.

And I'd take commit rights, but I can't promise I'll review anything. I'm a couple hundred reviews behind on sklearn ;)

Excellent, team made (https://github.com/orgs/numpy/teams/numpydoc-devs) and invite sent. And yeah, totally understand about no commitments. I suspect we're all in the same boat here:)

@jnothman
Copy link
Member

Merge?

@amueller
Copy link
Contributor Author

I think consensus was merge and we iterate?

@amueller
Copy link
Contributor Author

@jnothman you should have merge rights, right? (I can't but maybe there is a "can't merge own PRs setting?")

@jnothman
Copy link
Member

jnothman commented Sep 10, 2017 via email

@amueller
Copy link
Contributor Author

Oh I thought @rgommers gave us rights, but maybe not?

@rgommers
Copy link
Member

Oh I thought @rgommers gave us rights, but maybe not?

I asked both of you, and you said yes. @jnothman didn't respond so I didn't give him the rights. Very happy to do so now though. Do you want them @jnothman?

@jnothman
Copy link
Member

jnothman commented Sep 11, 2017 via email

@rgommers
Copy link
Member

Awesome! Invite sent.

@jnothman jnothman merged commit 449d789 into numpy:master Sep 17, 2017
@amueller
Copy link
Contributor Author

thanks :)

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.

4 participants