#362: add basic rst template for api documentation #385

nmm0 · 2023-05-22T23:30:58Z

Closes #362

ajpowelsnl · 2023-05-23T15:20:25Z

templates/api.rst

+    - for subsections
+
+    ^ for subsubsections
+


There are often several ways for doing basic things (e.g., creating subsections). Is there a reason to prefer one way over another?

Yeah this is what's recommended, I just copy and pasted from https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections

ajpowelsnl

This looks really good Nic. Many thanks for your help with this!

ajpowelsnl · 2023-05-23T15:46:58Z

templates/api.rst

+  Use the following convention for headings:
+
+    # with overline, for parts
+


A general comment: I'm not familiar with some of the "Sphinx-isms," and other might not be, either.
For example, what is a "part" vs. a "chapter" (operationally speaking), and what does "overline" mean in both contexts?

Would it help either in the README or here to point people to the Sphinx Style Guide: https://documentation-style-guide-sphinx.readthedocs.io/en/latest/style-guide.html

Hmm, this isn't exactly official and has some things like saying to use .txt extensions that I don't agree with

templates/api.rst

ajpowelsnl · 2023-05-23T16:34:09Z

templates/api.rst

+..
+  It may be useful to also have examples for individual functions above.
+
+  Prefer examples to usage.


Maybe put this idea ("Prefer examples to usage") up top, as a point of general guidance. A possible re-work of this sentence for clarity: "Prefer working, compile-able code to human-language description."

I tried to make it clearer, but I think it should go further down rather at the top. Maybe we could have a more "general guidelines" page?

ajpowelsnl · 2023-05-23T16:35:06Z

templates/api.rst

+
+  #include<Kokkos_Core.hpp>
+  #include<cstdio>
+


TODO: Amy, Nic + Francesco figure out Sphinx feature for correct rendering of C++ include statements

@fnrizzi -- can you help w/ this rendering question?

i don't think we have a way around, the include is inside the code block

ajpowelsnl · 2023-05-23T16:39:33Z

templates/api.rst

+     }
+
+     Kokkos::finalize();
+  }


Meeting comments:

In the general example, add all possible "Kokkos-isms" so everyone can understand the available customizations and how to use them

Add template for classes to facilitate copy / paste

Fix header rendering

Create compiled version (Daniel A.)

Add Kokkos to Compiler Explorer, and make examples work interactively with it (Cezary S.)

Submit issue to Compiler Explorer to add Kokkos (done)

Submit an issue to Compiler Explorer

From Compiler Explorer site:

``` “To add a library, search for one you want and select the version in the dropdown. Or if you have favorited it before, just click the library name in the Favorites section. Libraries are installed |using the conan.io package manager, except for Microsoft compilers, where vcpkg is used.” ```

ajpowelsnl

Hey Nic -- It generally looks good, but a few changes might help (see embedded comments).

ajpowelsnl · 2023-05-23T16:41:20Z

templates/api.rst

+     }
+
+     Kokkos::finalize();
+  }


WHOOPSIE! I missed this TODO:

Add example to "Contributing" pages as a compiled example

"Contributing"

I added the template to "Contributing". I actually made a folder that we can add more templates to

ajpowelsnl · 2023-06-07T23:15:52Z

templates/api.rst

+
+  #include<Kokkos_Core.hpp>
+  #include<cstdio>
+


@fnrizzi -- can you help w/ this rendering question?

templates/api.rst

nmm0 · 2023-07-24T23:06:52Z

I rebased and updated with some extra useful comments.

fnrizzi

@nmm0 can you please post also in the PR a screenshot of what this looks like?

templates/api.rst

fnrizzi · 2023-06-08T07:15:58Z

templates/api.rst

+
+  #include<Kokkos_Core.hpp>
+  #include<cstdio>
+


i don't think we have a way around, the include is inside the code block

fnrizzi · 2023-07-25T15:19:27Z

docs/source/templates/class_api.rst

+..
+  Use the following convention for headings:
+
+    # with overline, for parts


what are "parts"?

I clarified this

docs/source/templates/class_api.rst

ajpowelsnl

Hi Nic, Apologies for the late-in-the-day turn around, but there are a few things that are not clear to me. I'll be happy to work through these together.

ajpowelsnl · 2023-07-25T15:17:54Z

docs/source/templates/class_api.rst

+
+  ..
+    Template parameters (if applicable)
+    Omit template parameters that are just used for specialization/is deduced/ and/or should not be exposed to the user.


Please change "is deduced" to "are deduced" for subject - verb agreement.

ajpowelsnl · 2023-07-25T15:20:37Z

docs/source/templates/class_api.rst

+
+  .. rubric:: Template Parameters
+
+  :tparam Foo: Description of the Foo template parameter


Please make "Foo" capitalization consistent with "bar" ...either way is fine.

Here it's just our coding convention of capitalizing template parameters and having parameters/locals lowercase. So the documentation just follows the main core Kokkos coding convention

ajpowelsnl · 2023-07-25T15:24:13Z

docs/source/templates/class_api.rst

+      If you have related info
+
+    .. seealso::
+


With this, is your idea to link within or among other documentation entries?

Both -- cross referencing works across files. They can even be namespaced which is nice

ajpowelsnl · 2023-07-25T21:53:48Z

docs/source/templates/class_api.rst

+
+  .. cppkokkos:function:: ~CoolerView()
+
+    Document what special effect the destructor has.


This directive, "Document what special ..." is a bit conflict-y with the comment above. A few questions:

The special effects you mention are in cases apart from the usual business of normal destructor housekeeping ?

Are there common, canonical examples of special destructor effects? If so, maybe mention some of those to firm up guidance.

See the comment -- special effects include things like releasing resources, etc. I mention in the comments

ajpowelsnl · 2023-07-25T22:00:18Z

docs/source/templates/class_api.rst

+
+    .. versionchanged:: 3.7.1
+
+      Only takes one parameter for foo-style operations instead of two.


I don't quite understand line 116. Is this phrase in relation to .. versionchanged:: 3.7.1, or something else? Either way, I don't understand what you're telling me here.

Yes, rst is a little bit annoying to read but if you have a directive (.. directive:: where directive is the name of the directive). After the two colons are the arguments to the directives, so in this case the version in which it was changed (3.7.1). Some directives have "options" (not this one) and "contents". The "options" and "contents" are tabbed (tabbing is important!) and an empty line must be before the "contents". So in this case the string Only takes one parameter for foo-style operations instead of two. is the contents of the directive because it's tabbed over one and represents the blurb that gets rendered by sphinx.

Annoyingly, the definition for a comment is similar (there is just no double colon). To make it less confusing it's commonly recommended to put the comment on the next line after the ..

.. This is a comment.

Here is some referenc: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#directives

ajpowelsnl · 2023-07-25T22:01:59Z

docs/source/templates/class_api.rst

+    Deprecated types need special handling, both for the badge displayed beforehand and the deprecated version directive.
+    Note, there is not currently any functionality for other deprecated entities.
+
+  .. cppkokkos:deprecated-type:: 4.0.1 foobar


In your comment, maybe note key points of our discussion this morning about deprecation syntax in .rst files, and (parser) rendering in html.

Done! Though I did move to the c++ style [[deprecated("reason")]]

ajpowelsnl · 2023-07-25T22:14:32Z

docs/source/templates/class_api.rst

+
+..
+  These should only be listed here if they are closely related. E.g. friend operators. However,
+  something like view_alloc shouldn't be here for view


Can we readily identify those "friends?" Is there a Sphinx cpp customization to flag "friend" methods, operators, etc.?

No sadly, but at least having them logically grouped in the same documentation makes sense.

ajpowelsnl · 2023-07-25T22:16:19Z

docs/source/templates/class_api.rst

+  something like view_alloc shouldn't be here for view
+
+.. cppkokkos:function:: template<class ViewSrc> bool operator==(CoolerView, ViewSrc);
+


Is this an actual comparator? i.e., something testing that CoolerView is the same as ViewSrc, or is this just your example of an "allied" or "friend" operator?

Just an example

ajpowelsnl · 2023-07-25T22:28:56Z

docs/source/templates/class_api.rst

+  :tparam ViewDst: the other
+
+  :return: true if :cppkokkos:type:`~View::value_type`, :cppkokkos:type:`~View::array_layout`, :cppkokkos:any:`~View::memory_space`, :cppkokkos:any:`~View::rank`, :cppkokkos:any:`~View::data()` and :cppkokkos:any:`~View::extent` (r), for :code:`0<=r<rank`, match.
+


For lines 146 - 148, a few questions:

what is ViewDst the template parameter for? I don't see it in any near-by functions that use it.

what do the ~ represent in line 148?

is everything in lines 146 - 148 meant to be used in the operator in line 144? I'm having trouble seeing the flow of events : )

This is just an example. I mostly copied it from the view documentation.

The ~ is a typo, it shouldn't be in there, I removed now. These are all supposed to be cross-references, but right now we don't document View as a class so they won't work. Yes, this is all the documentation for the operator.

ajpowelsnl · 2023-07-25T22:30:01Z

docs/source/templates/class_api.rst

+  It may be useful to also have examples for individual functions above.
+
+  Prefer working and compilable examples to prose descriptions (such as "Usage").
+


I'll check to make sure this code compiles in Compiler Explorer.

fnrizzi · 2023-08-01T06:01:32Z

docs/source/templates/class_api.rst

+  for free functions that are callable, preserve the naming convention, `view_alloc()`
+
+``CoolerView``
+=====================


the length of === should be fixed to match the word otherwise this is misleading since sphinx would raise an exception for this

fnrizzi · 2023-08-01T06:02:04Z

docs/source/templates/class_api.rst

+
+
+Non-Member Functions
+----------------------


length of ---- should end under s of Functions

fnrizzi · 2023-08-01T06:03:00Z

docs/source/contributing.rst

+   :hidden:
+
+   templates/index
+
 We are open and try to encourage contributions from external developers.
 To do so please first open an issue describing the contribution and then
 issue a pull request against the develop branch.


there is no develop branch, ah nevermind this is for actual code in core

fnrizzi

just a few minor ones

docs/source/templates/class_api.rst

nmm0 · 2023-08-15T15:31:00Z

I don't know why the build is failing but main is not. I haven't touched the failing file (keywords.rst)

… and examples should be used

…be found inside of the Contributing page

nliber · 2023-09-21T23:06:49Z

docs/source/templates/class_api.rst

+    :language: cppkokkos
+
+..
+  The (pulic header) file the user will include in their code


Suggested change

The (pulic header) file the user will include in their code

The (public header) file the user will include in their code

nmm0 changed the title ~~#362: add basic rst template for api documentation~~ Draft: #362: add basic rst template for api documentation May 22, 2023

nmm0 marked this pull request as draft May 22, 2023 23:31

vlkale marked this pull request as ready for review May 22, 2023 23:38

ajpowelsnl reviewed May 23, 2023

View reviewed changes

ajpowelsnl requested changes May 23, 2023

View reviewed changes

ajpowelsnl requested changes Jun 7, 2023

View reviewed changes

fnrizzi reviewed Jun 8, 2023

View reviewed changes

templates/api.rst Outdated Show resolved Hide resolved

ajpowelsnl mentioned this pull request Jun 30, 2023

Updated RangePolicy documentation #428

Closed

ajpowelsnl requested review from crtrott and ldh4 July 9, 2023 17:45

nmm0 force-pushed the add-documentation-template branch from 26739a2 to d863aee Compare July 24, 2023 22:32

nmm0 changed the title ~~Draft: #362: add basic rst template for api documentation~~ #362: add basic rst template for api documentation Jul 24, 2023

fnrizzi requested changes Jul 25, 2023

View reviewed changes

ajpowelsnl requested changes Jul 25, 2023

View reviewed changes

ajpowelsnl requested a review from dalg24 July 31, 2023 15:48

nmm0 requested review from fnrizzi and ajpowelsnl July 31, 2023 18:21

fnrizzi reviewed Aug 1, 2023

View reviewed changes

fnrizzi requested changes Aug 1, 2023

View reviewed changes

ajpowelsnl mentioned this pull request Aug 15, 2023

add rst templates for adding documentation #362

Closed

3 tasks

crtrott requested changes Aug 15, 2023

View reviewed changes

docs/source/templates/class_api.rst Outdated Show resolved Hide resolved

docs/source/templates/class_api.rst Outdated Show resolved Hide resolved

nmm0 force-pushed the add-documentation-template branch from 4972691 to b87a5eb Compare August 15, 2023 15:25

nmm0 added 4 commits August 15, 2023 08:42

add basic rst template for api documentation

3833072

fix some syntax issues and clarify some of the times where destructor…

3e6647c

… and examples should be used

add .venv to gitignore

5adf4e2

move template to inside the source directory

7a9fafd

nmm0 and others added 5 commits August 15, 2023 08:42

organize templates so they are rendered by the build process and can …

c77c4ad

…be found inside of the Contributing page

document versionchanged, etc

a5e5f81

use c++-style deprecated and address some PR feedback

d5de468

fix some typos and update the comment on deprecated

f6ff1ee

Apply suggestions from code review

fd37482

nmm0 force-pushed the add-documentation-template branch from b87a5eb to fd37482 Compare August 15, 2023 15:42

ajpowelsnl approved these changes Aug 15, 2023

View reviewed changes

nmm0 requested review from crtrott and fnrizzi August 15, 2023 15:49

crtrott approved these changes Aug 15, 2023

View reviewed changes

crtrott merged commit 52443dd into kokkos:main Aug 15, 2023
1 check passed

nliber reviewed Sep 21, 2023

View reviewed changes

		Use the following convention for headings:

		# with overline, for parts


		.. rubric:: Template Parameters

		:tparam Foo: Description of the Foo template parameter


		.. cppkokkos:function:: ~CoolerView()

		Document what special effect the destructor has.


		.. versionchanged:: 3.7.1

		Only takes one parameter for foo-style operations instead of two.

		something like view_alloc shouldn't be here for view

		.. cppkokkos:function:: template<class ViewSrc> bool operator==(CoolerView, ViewSrc);

		:tparam ViewDst: the other

		:return: true if :cppkokkos:type:`~View::value_type`, :cppkokkos:type:`~View::array_layout`, :cppkokkos:any:`~View::memory_space`, :cppkokkos:any:`~View::rank`, :cppkokkos:any:`~View::data()` and :cppkokkos:any:`~View::extent` (r), for :code:`0<=r<rank`, match.

		It may be useful to also have examples for individual functions above.

		Prefer working and compilable examples to prose descriptions (such as "Usage").

	The (pulic header) file the user will include in their code
	The (public header) file the user will include in their code



		Non-Member Functions
		----------------------

#362: add basic rst template for api documentation #385

#362: add basic rst template for api documentation #385

Conversation

nmm0 commented May 22, 2023

ajpowelsnl May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajpowelsnl left a comment

Choose a reason for hiding this comment

ajpowelsnl May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajpowelsnl May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajpowelsnl May 23, 2023 • edited Loading

Choose a reason for hiding this comment

ajpowelsnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmm0 commented Jul 24, 2023

fnrizzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajpowelsnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fnrizzi Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

fnrizzi Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

fnrizzi left a comment

Choose a reason for hiding this comment

nmm0 commented Aug 15, 2023

Choose a reason for hiding this comment

ajpowelsnl May 23, 2023 •

edited

Loading

ajpowelsnl May 23, 2023 •

edited

Loading

ajpowelsnl May 23, 2023 •

edited

Loading

ajpowelsnl May 23, 2023 •

edited

Loading

fnrizzi Aug 1, 2023 •

edited

Loading

fnrizzi Aug 1, 2023 •

edited

Loading