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

DOC: docstring for show_config. #9275

Closed
wants to merge 8 commits into from
Closed

DOC: docstring for show_config. #9275

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 21, 2017

A docstring for show_config (alias for the function show).

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @siryog90. In addition to my minor comments on content/formatting, it would be good to ensure that this actually ends up in the html docs. Probably the best place to put it is in the packaging section (although other suggestions welcome): file:///home/rgommers/Code/numpy/doc/build/html/reference/distutils.html

Edit doc/source/reference/distutils.rst to get it there. The ``get_include function can also be added at the same time, it's not present in the docs under that name


Examples
--------
>>> import numpy as np
Copy link
Member

Choose a reason for hiding this comment

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

this import isn't needed, it's assumed in every example.

Examples
--------
>>> import numpy as np
>>> np.__config__.show()
Copy link
Member

Choose a reason for hiding this comment

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

Should be np.show_config because that's the public function.

Notes
-----
Classes specifying the information to be printed are defined
in the ``numpy.distuitls.system_info`` module.
Copy link
Member

Choose a reason for hiding this comment

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

  • typo in distuilts
  • use single backticks, then it will be rendered as a link

Classes specifying the information to be printed are defined
in the ``numpy.distuitls.system_info`` module.

Information may include:
Copy link
Member

Choose a reason for hiding this comment

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

Needs a blank line here I think, for the list to render correctly

in the ``numpy.distuitls.system_info`` module.

Information may include:
* language: language used to write the libraries (mostly C or f77)
Copy link
Member

Choose a reason for hiding this comment

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

suggest to use double backticks around each name before the colon, so those are typeset as code.


Information is grouped according to classes (``blas_opt_info`` in the example)
that specify relevant resources, and it is formatted to be compatible with
``distutils.setup`` keyword arguments.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this last sentence is correct, there are no such keywords for setup. They are there for add_library and add_extension though, so I guess that's what you meant. It's not super important I think, so suggest moving to the end of the Notes section.

@ghost
Copy link
Author

ghost commented Jun 21, 2017

@rgommers

I think it may be better to create a Build system info functions (maybe a more appropriate name?) subsection of routines and put the documentation there, since show_config isn't really called as part of the distutils package. Thoughts?

Edit: Better yet, how about putting it in Miscellaneous routines ?

@ghost ghost changed the title docstring for show_config. DOC: docstring for show_config. Jun 25, 2017
@rgommers
Copy link
Member

Docstring looks good to me now. Miscellaneous routines seems like a good place.

You still have to add show_config to the rendered docs, putting it next to get_include would make sense to me.

@ghost
Copy link
Author

ghost commented Jun 25, 2017

@rgommers
Take a look when you get a chance.

The docs say that autosummary is supposed to create a table with the first line of a function's description next to it's signature. Not exactly sure why this doesn't happen for show_config. There is no docstring per se for autosummary to use, but I am guessing the docstring would just get translated to Sphinx directives like what I have in numpy.show_config.rst

Do I need to make the table manually?

@ghost ghost closed this Jun 26, 2017
@ghost ghost deleted the show_config_docstring branch June 26, 2017 01:21
@charris
Copy link
Member

charris commented Jun 26, 2017

Hmm...

@rgommers
Copy link
Member

@siryog90 what's going on? You were close to done, and now it looks like you've deleted your branch.

@ghost
Copy link
Author

ghost commented Jun 27, 2017

@rgommers @charris
Finished it.
I was having some issues with the branch, so just checked out a new one.
About to make a pull request for a review.

@ghost ghost mentioned this pull request Jul 1, 2017
This pull request was closed.
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.

2 participants