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: Add support for documenting C/C++ via Doxygen & Breathe #18884
Conversation
7cab7f7
to
515cd8d
Compare
14d7a84
to
79b721a
Compare
6722d3d
to
64382e2
Compare
64382e2
to
73d8d7f
Compare
9123690
to
d4f7330
Compare
@seiko2plus Where does the new documentation go and how is it run. Perhaps that information could be included in the release note. |
This patch doesn't generate any documentation from the sources, just initialize Doxygen and Breathe. see |
doc/source/docs/howto_document.rst
Outdated
This is how Javadoc style looks like:: | ||
|
||
/** | ||
* Your brief is here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Your brief is here. | |
* function(void) changes everything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the point behind this suggestion, any reason?
doc/source/docs/howto_document.rst
Outdated
|
||
For line comment, insert a triple forward slash:: | ||
|
||
/// Your brief is here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Your brief is here. | |
/// function(void) changes everything. |
doc/source/docs/howto_document.rst
Outdated
|
||
Common Doxygen Tags: | ||
++++++++++++++++++++ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we take a stand on common formatting? Doxygen takes @b BOLD
and some others, but also other styles (like rst and markdown).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about extending this doc to cover format styles, I think it's not necessary the reader can always check doxygen docs. note during the last rebase I had to move the new changes to source/dev/howto-docs.rst
@mattip, Yes, I think it would be possible to check the existence of |
198b423
to
e8d0a73
Compare
|
||
Starts a paragraph that serves as a brief description. By default the first sentence | ||
of the documentation block is automatically treated as a brief description, since | ||
option `JAVADOC_AUTOBRIEF <https://www.doxygen.nl/manual/config.html#cfg_javadoc_autobrief>`__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to prefer JAVADOC_AUTOBRIEF
over requiring explicit @brief
and @details
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, there's no way to force Doxygen to require @brief
and @details
. If JAVADOC_AUTOBRIEF
is off it will never interpret parsing the brief until the first empty line. Why we should require @brief and @details
in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, if JAVADOC_AUTOBRIEF is OFF, then it will follow the Qt style.
doc/source/dev/howto-docs.rst
Outdated
|
||
For line comment, insert a triple forward slash:: | ||
|
||
/// Your brief is here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line comments are in most cases used wrongly. They get added to the @details
block or the @brief
block, but out of order. Is there a good use case for not documenting the function only at the toplevel? Additionally this is visually harder to notice compared to the full style.
/**
* @brief This function does something
* @details It is a good function. The concept behind it is the same as the (ref) algorithm
* @param Nothing
* @returns Nothing
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until the first dot, it goes to @brief. Line comment is very useful for C++, see the following example
/**
* Template to represent limbo numbers.
*
* Specializations for integer types that are part of nowhere.
* It doesn't support with any real types.
*
* @param Tp Type of the integer. Required to be an integer type.
* @param N Number of elements.
*/
template<typename Tp, std::size_t N>
class DoxyLimbo {
public:
/// Default constructor. Initialize nothing.
DoxyLimbo();
/// Set Default behavior for copy the limbo.
DoxyLimbo(const DoxyLimbo<Tp, N> &l);
/// Returns the raw data for the limbo.
const Tp *data();
/// @name Comparison operators.
/// @{
bool operator==(const DoxyLimbo<Tp, N> &r) const;
bool operator<(const DoxyLimbo<Tp, N> &r) const;
bool operator<=(const DoxyLimbo<Tp, N> &r) const;
/// @}
protected:
Tp p_data[N]; ///< Just C array that hold the limbos.
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a stylistic issue. I personally feel that most new contributors would benefit from the explicit tags, since I do not expect them to know the moires of Doxygen. I also find it more evident (i.e. it is clearer when something is going to be processed by doxygen
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think documenting C-API interfaces is something we want new contributors to take on until there is a good reference of already-documented interfaces they can copy from. This is the "howto docs" page, we could start another page for a more detailed style guide if needed. I think we should aim for a bare minimum in this PR so we can get it approved and merged, we can always expand in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a stylistic issue
That's why I added the following statement at the beginning:
Although there is still no commenting style set to follow, the Javadoc
is more preferable than the others due to the similarities with the current
existing non-indexed comment blocks.
And since tags brief
and details
aren't part of Javadoc style then we shoudn't require it.
We currently use Sphinx_ along with Doxygen_ for generating the API and | ||
reference documentation for NumPy. In addition, building the documentation | ||
requires the Sphinx extension `plot_directive`, which is shipped with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since make html
will still work without it; consider splitting into two sentences.
We currently use Spinx_ to generate most of the API documentation.
However, the C-Numpy API documentation is generated with Doxygen_.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there is some misunderstanding. Doxygen will not be used alone to generate the C-API documentation, but perhaps we can revisit this in the future. This PR implements using Doxygen to pull formatted comments out of the C code and turn it into XML, then uses breeze to turn that XML into reST, then uses Sphinx to turn the reST into HTML/PDF/Latex. So the second proposed sentence is less correct than the original.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the end users point of view, isn't the current implementation expected to throw warnings for the C-API sections of the documentation and otherwise work as is? From this perspective it makes sense to explain the change in a minimally invasive manner here while leaving the details:
Comments->XML->RST->Combined docs
can be elsewhere. No strong feelings about this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Inclusion directives is pretty clear I guess:
Breathe_ provides a wide range of custom directives to allow
converting the documents generated by Doxygen_ into reST files.
Do you think it isn't clear enough? the reader should check Breathe docs after all.
We discussed this at the last documentation meeting. The general opinion was that the increased complexity is worth the benefit. We should encourage people wishing to contribute to the docs to use conda or gitpod if they find it difficult to set up their environment (the `environment.yml file should be adapted to install Doxygen). There was a discussion about using a direct source -> Doxygen -> HTML/PDF/Latex build step, since then we would get things like "link to source", but we decided to proceed with this path for now. We can always revisit this decision once we have a reasonable quantity of commented source code. |
There is no change to how to run documentation builds: it is still The documention will be pulled in anywhere we use a special breathe directives like |
961e431
to
3425729
Compare
Co-authored-by: Matti Picus <matti.picus@gmail.com>
…eathe isn't available.
3425729
to
defd9fe
Compare
3eeb3d8
to
241527a
Compare
Examples have been improved, Doxygen and Breathe are no longer required the build can pass without them. Current look: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for addressing my comments.
I think we should put this in as-is, and in a future PR move the details to a separate page. @melissawm could you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I build the docs locally quite frequently, so I wanted to see what impact this would have on the doc-building workflow. I built in a few different environments:
- neither doxygen nor breathe installed
- breath installed, but not doxygen (this scenario might affect pip users)
- both breathe and doxygen installed.
The two main things I was keeping an eye out for were a) any new warnings/errors and b) if there were any impacts on timing.
Warnings
In envs 1 & 2, where either one or the other of breathe/doxygen was not installed, I noticed three new warnings during the sphinx build. These are just ordinary sphinx warnings that don't interrupt the build process at all. In env 3 where both doxygen+breathe are installed, there are no new sphinx warnings, but there is a lot of new output from doxygen itself that precedes the typical sphinx output. See the details block below for an example of the specific output (which included doxygen warnings in my case). This also doesn't seem to interfere with the docs build process at all.
New doxygen output
# for testing # @echo installed numpy 241527a3cb matches git version 241527a3cb; exit 1 mkdir -p build touch build/generate-stamp mkdir -p build/html build/doctrees python3.9 preprocess.py doxygen build/doxygen/Doxyfile warning: Tag 'OUTPUT_TEXT_DIRECTION' at line 15 of file 'build/doxygen/Doxyfile' has become obsolete. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u" warning: Tag 'CLANG_ASSISTED_PARSING' at line 149 of file 'build/doxygen/Doxyfile' belongs to an option that was not enabled at compile time. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled. warning: Tag 'CLANG_OPTIONS' at line 150 of file 'build/doxygen/Doxyfile' belongs to an option that was not enabled at compile time. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled. warning: Tag 'CLANG_DATABASE_PATH' at line 151 of file 'build/doxygen/Doxyfile' belongs to an option that was not enabled at compile time. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u", or recompile doxygen with this feature enabled. warning: Tag 'COLS_IN_ALPHA_INDEX' at line 156 of file 'build/doxygen/Doxyfile' has become obsolete. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u" warning: Tag 'LATEX_SOURCE_CODE' at line 238 of file 'build/doxygen/Doxyfile' has become obsolete. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u" warning: Tag 'RTF_SOURCE_CODE' at line 251 of file 'build/doxygen/Doxyfile' has become obsolete. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u" warning: Tag 'DOCBOOK_PROGRAMLISTING' at line 272 of file 'build/doxygen/Doxyfile' has become obsolete. To avoid this warning please remove this line from your configuration file or upgrade it using "doxygen -u" Doxygen version used: 1.9.2 Searching for include files... Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd/neon Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd/vsx Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd/avx2 Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd/sse Searching for files in directory /home/ross/repos/numpy/numpy/core/src/common/simd/avx512 Searching for files in directory /home/ross/repos/numpy/numpy/core/include/numpy Searching for files in directory /home/ross/repos/numpy/numpy/core/include/numpy/random Searching for files in directory /home/ross/repos/numpy/numpy/core/include/numpy/libdivide Searching for files in directory /home/ross/repos/numpy/doc/source/dev/examples Searching for example files... Searching for images... Searching for dot files... Searching for msc files... Searching for dia files... Searching for files to exclude Searching INPUT for files to process... Searching for files in directory /home/ross/repos/numpy/doc/source/dev/examples Reading and parsing tag files Parsing files Preprocessing /home/ross/repos/numpy/doc/source/dev/examples/doxy_class.hpp... Parsing file /home/ross/repos/numpy/doc/source/dev/examples/doxy_class.hpp... Preprocessing /home/ross/repos/numpy/doc/source/dev/examples/doxy_func.h... Parsing file /home/ross/repos/numpy/doc/source/dev/examples/doxy_func.h... Preprocessing /home/ross/repos/numpy/doc/source/dev/examples/doxy_rst.h... Parsing file /home/ross/repos/numpy/doc/source/dev/examples/doxy_rst.h... Building macro definition list... Building group list... Building directory list... Building namespace list... Building file list... Building class list... Building concept list... Computing nesting relations for classes... Associating documentation with classes... Associating documentation with concepts... Building example list... Searching for enumerations... Searching for documented typedefs... Searching for members imported via using declarations... Searching for included using directives... Searching for documented variables... Building interface member list... Building member list... Searching for friends... Searching for documented defines... Computing class inheritance relations... Computing class usage relations... Flushing cached template relations that have become invalid... Computing class relations... Add enum values to enums... Searching for member function documentation... Creating members for template instances... Building page list... Search for main page... Computing page relations... Determining the scope of groups... Sorting lists... Determining which enums are documented Computing member relations... Building full member lists recursively... Adding members to member groups. Computing member references... Inheriting documentation... Generating disk names... Adding source references... Adding xrefitems... Sorting member lists... Setting anonymous enum type... Computing dependencies between directories... Generating citations page... Counting members... Counting data structures... Resolving user defined references... Finding anchors and sections in the documentation... Transferring function references... Combining using relations... Adding members to index pages... Correcting members for VHDL... Computing tooltip texts... Generating style sheet... Generating search indices... Generating example documentation... Generating file sources... Generating code for file doc/source/dev/examples/doxy_class.hpp... Generating code for file doc/source/dev/examples/doxy_func.h... Generating code for file doc/source/dev/examples/doxy_rst.h... Generating file documentation... Generating page documentation... Generating group documentation... Generating class documentation... Generating docs for compound DoxyLimbo... Generating concept documentation... Generating namespace index... Generating graph info page... Generating directory documentation... finalizing index lists... writing tag file... Generating XML output... Generating XML output for class DoxyLimbo Generating XML output for file doxy_class.hpp Generating XML output for file doxy_func.h Generating XML output for file doxy_rst.h Generate XML output for dir /home/ross/repos/numpy/doc/source/dev/ Generate XML output for dir /home/ross/repos/numpy/doc/ Generate XML output for dir /home/ross/repos/numpy/doc/source/dev/examples/ Generate XML output for dir /home/ross/repos/numpy/doc/source/ Running plantuml with JAVA... lookup cache used 5/65536 hits=13 misses=5 finished... LANG=C sphinx-build -b html -WT --keep-going -d build/doctrees source build/html Running Sphinx v4.1.1 1.22.dev0 1.22.0.dev0+968.g241527a3c
Performance
Since this PR doesn't include a whole bunch of new auto-generated C documentation yet, I would expect the doc build time not to be affected much. I was able to confirm this with some very rudimentary benchmarking (time make html
) in all 3 environments. I essentially saw no difference in total build time (~5min 15 sec on my laptop in all 3 environments).
So IMO this seems to have a minimal impact on the current doc building workflow. Things get a little noisier, either in the form of new sphinx warnings or doxygen output, but there don't seem to be any unexpected side effects (at least in the things I was paying attention to). For reference this was on Linux with doxygen
installed by the system package manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My previous comment was supposed to include an approval - sorry for the noise.
Thanks @seiko2plus |
Add support for documenting C/C++ via Doxygen & Breathe
Due to the urgent need to generate C/C++ API reference documentation from comment blocks, especially with
the most recent expansion in the use of SIMD, and also due to the future reliance on C++.
TODO: