#166 Custom sphinx directive for deprecating a type alias #172

marcinwrobel1986 · 2022-09-22T06:14:29Z

THIS PR:

add cppkokkos extension to Sphinx
add Sphinx domain - cppkokkos
set CppLexer as default lexer for cppkokkos domain
add deprecated-type directive to cppkokkos domain with [DEPRECATED] in front
add support for version in deprecated-type, [DEPRECATED since 3.6.01]
removed type in front of the defined type in Sphinx extension
modified the edit_button_handler script, so it change color of deprecated type with/without version

Test page look:

The `*.rst` syntax:

``Kokkos::TestView``
====================

.. role:: cppkokkos(code)
   :language: cppkokkos

Header File: ``Kokkos_Core.hpp``

Class Interface
---------------

.. cppkokkos:class:: template <class DataType, class... Traits> TestView

  A potentially reference counted multi dimensional array with compile time layouts and memory space.
  Its semantics are similar to that of :cppkokkos:`std::shared_ptr`.

  .. rubric:: Public Member Variables

  .. cppkokkos:member:: static constexpr unsigned rank

    the rank of the view (i.e. the dimensionality).

  .. cppkokkos:type:: const_data_type

    The const version of ``DataType``, same as ``data_type`` if that is already const.

  .. cppkokkos:type:: uniform_runtime_type

    :cppkokkos:`uniform_type` but without compile time extents

  .. cppkokkos:deprecated-type:: 3.6.01 some_deprecated_data_type

    The const version of some_deprecated_data_type.

  .. cppkokkos:deprecated-type:: other_deprecated_data_type

    The const version of other_deprecated_data_type.

  .. cppkokkos:deprecated-type:: 3.7.0 some_deprecated_data_type_1

    The const version of some_deprecated_data_type_1.

  .. cppkokkos:function:: View( const ScratchSpace& space, const IntType& ... indices)

    *Requires:* :cppkokkos:`sizeof(IntType...)==rank_dynamic()` *and*  :cppkokkos:`array_layout::is_regular == true`.

    A constructor which acquires memory from a Scratch Memory handle.

    :param space: a scratch memory handle. Typically returned from :cppkokkos:`team_handles` in :cppkokkos:`TeamPolicy` kernels.
    :param indices: the runtime dimensions of the view.

- created cppkokkos based on kokkos - added view_test.rst in order to see how cppkokkos works

- removed `type` word before actual type in html

- supports regex from kokkos releases - added support for changing colors in edit_button_handler - added deprecation type with version to view_test.rst

marcinwrobel1986 · 2022-09-26T07:16:16Z

Hello @crtrott @dalg24 @JBludau @ajpowelsnl @nmm0 @fnrizzi ,
I was working pretty hard, for this PR. I will be showing that on the meeting, but I want to give you some piece of information before that.

So, as @nmm0 suggested (thanks for that BTW) I checked the Sphinx extension topic. As it appeared it was not that easy, but doable.
This extension give us some freedom in customization as well as we don't have to change the Sphinx code in any way, which is a huge win.

In order for this extension to work I created another domain - I called it cppkokkos (this can be changed, but seems appropriate), it is basically same as cpp, but with all custom features. To this newly created domain I plugged in cpp syntax highlighting, so everything works like in cpp domain.

In order to remove type in front of the data type I customized cppkokkos domain. I added support for deprecated-type with and without specific version - so it could display [DEPRECATED] or [DEPRECATED since 3.7.0] when version is given. I also added support to the post processing script to change style for deprecated type.

JBludau · 2022-09-26T14:38:54Z

this is pretty cool, thanks for the good work.
As it is a type, I guess we can annotate functions (via the return type). But can we also use it to mark constructors as deprecated?

nmm0 · 2022-09-26T15:38:15Z

Nice, I like how it looks and how easy it is to use!

marcinwrobel1986 · 2022-09-27T08:09:01Z

As it is a type, I guess we can annotate functions (via the return type). But can we also use it to mark constructors as deprecated?

@JBludau at the moment we have defined an extra directive deprecated-type, which can take version number. We can define anything regarding functions/classes as now we have an extension, which is customizable. Please define what and how should we customize or maybe we should define a new type e.g. deprecated-function?

JBludau · 2022-09-27T14:17:02Z

@marcinwrobel1986 we had an internal discussion a while ago where we basically identified (if I remember correctly, please correct me if I am wrong) that we would want the annotations Since version xx Until version xx and Deprecated since xx (like you already have shown). These would apply to anything (class, function, type, CMAKE variable)

marcinwrobel1986 · 2022-09-28T06:52:27Z

@marcinwrobel1986 we had an internal discussion a while ago where we basically identified (if I remember correctly, please correct me if I am wrong) that we would want the annotations Since version xx Until version xx and Deprecated since xx (like you already have shown). These would apply to anything (class, function, type, CMAKE variable)

Hello @JBludau ,
The discussion was in HERE
You mentioned there:

promoted -> things that are moved out of Experimental into the main namespace in a version.
deprecated -> things that are deprecated in ... and will be removed in ...
since and until -> not sure if we need a until it is similar to deprecated, but a since would make sense for newly introduced things
behavior change -> hint on a change of behavior in a certain version (e.g. commandline argument parsing)

So for me now, when we have this extension it shouldn't be a problem to add anything.
IMHO what would make sense it to create:

experimental if you have such a class, function, type to mark it laud and clearly, that it is experimental
deprecated, deprecated since ... - for deprecated class, function, type
since if one wants to communicate to user since which version is that class, function, type available

I think there is no need for until. The deprecated and deprecated since ... could be use instead, but this is just my opinion, I will do whatever you want guys.

JBludau · 2022-10-03T14:59:45Z

Hello @JBludau , The discussion was in HERE You mentioned there:

* `promoted` -> things that are moved out of `Experimental` into the main namespace in a version.

* `deprecated` -> things that are deprecated in ... and will be removed in ...

* `since and until` -> not sure if we need a `until` it is similar to `deprecated`, but a `since` would make sense for newly introduced things

* `behavior change` -> hint on a change of behavior in a certain version (e.g. commandline argument parsing)

@marcinwrobel1986 I remember that we had a later discussion in the documentation session on this, thus this is not the latest info. @ajpowelsnl can you point us to the notes?
I think the bottom line was: since and until would be enough to cover almost everything. I don't remember but thought we wanted deprecated for convenience. For a behavior change we would be fine with the warning box that we already have. The question how to mark something as being promoted from experimental was not fully solved I think. But I think the easiest was marking it with since. Nevertheless, this leads to the problem I tried to describe in #167

Does this make sense?

ajpowelsnl

Looks good -- I just had the one small question about PATH'ing in the docs/source/conf.py file.

ajpowelsnl · 2022-10-04T23:12:54Z

docs/source/conf.py

@@ -12,6 +12,7 @@
 #
 import os
 import sys
+sys.path.append(os.path.abspath("./_ext"))
 sys.path.insert(0, os.path.abspath('.'))



Why are these PATH manipulations necessary, i.e., the sys.path .... statements?

Hello @ajpowelsnl docs/source/conf.py is a Sphinx config file. In this file one could customize Sphinx behavior. This is a Python file as well, so things defined there go through Python's interpreter.
Line sys.path.append(os.path.abspath("./_ext")) adds directory _ext to the path. This is where out extension cppkokkos is located (_ext/cppkokkos.py).
Extension is added a few lines below:

extensions = ["myst_parser", "sphinx.ext.autodoc", "sphinx.ext.viewcode", "sphinx.ext.intersphinx", "sphinx_copybutton", "cppkokkos"]

Summarizing: this allows us to use our custom extension cppkokkos with Sphinx, where new directives are added and other are customized.
I hope that answer your question.

ajpowelsnl · 2022-10-04T23:29:05Z

Another (hopefully) simple question: does adding the cppkokkos extension complicate / prevent updating the (core) Sphinx code regularly? In other words, we don't want to drift significantly from main (Sphinx) source code. Nice work, BTW!

marcinwrobel1986 · 2022-10-19T07:54:47Z

Another (hopefully) simple question: does adding the cppkokkos extension complicate / prevent updating the (core) Sphinx code regularly? In other words, we don't want to drift significantly from main (Sphinx) source code. Nice work, BTW!

@ajpowelsnl creating a Sphinx extension totally separate us the Sphinx code. OFC when there would be a major update to Sphinx like changing the version from 5.x.x to 6.x.x and we would like to use 6.x.x the extension compatibility must be check before migrating to 6.x.x. Apart from that there is no changes needed in Sphinx codebase or any PR's to Sphinx, which is a huge win. Oh, and thanks for kind words! :)

JBludau · 2022-10-19T15:19:33Z

@ajpowelsnl what @marcinwrobel1986 is saying is, that if Sphinx changes their public api we need to adapt our extension to it but as long as it stays stable we can update.

JBludau

I hope there is another way ...

JBludau · 2022-10-20T17:03:05Z

docs/source/_ext/cppkokkos.py

@@ -0,0 +1,7869 @@
+"""The C++ language domain."""


Where did this come from and why would we need to copy it ... is this their way of adding extensions? This will be bad ...

Furthermore, this is not reviewable ... 7k line change

@JBludau I took C++ domain and changed it to cppkokkos domain. This is adding stuff and changed behavior of existing stuff. This was the only way to do that. I don't expect anyone to review that extension.

in this case could you post a diff so we can see what you added?
If we stick with the copy we would need to invest this effort every time the original is updated ... do you know how stable it is?

@JBludau I will post a diff tomorrow morning CET.
I will check tomorrow as well compatibility with latest Sphinx. We use 5.0.1 (was the last version when we started our work) and the latest is 5.3
IMHO in Sphinx nomenclature this is just a Domain and it should not change in the future, maybe it could in case of some bugs. But it does not have to do much/anything with the Sphinx core.
The story is that I don't like duplicate code especially nearly 8k lines, but that was the only way we have a domain which works like C++ domain, but is customizable. One can not register twice domain or lexer with the same name. CppLexer is imported (not modified in any way), so here we will always be up to date. Back to the story I started slowly with smaller pieces, but I quickly figure out I need all of it.
Summarizing CPPKokkosDomain inherits from Sphinx's Domain, so in worst case scenario they could change the API in Sphinx 6.x.x, but I really doubt they will, because it would mean they have to change all the domains defined in Sphinx package + all user(customers) defined domains would need to be changed.

guess we are kind of stuck with copying the 7k lines then ...

There is OFC option to reinvent the wheel :)

marcinwrobel1986 · 2022-10-21T06:40:01Z

@JBludau I attached the diff here.
diff_cpp_domain.txt

marcinwrobel1986 · 2022-10-21T08:40:38Z

@JBludau I can also confirn, that the extension works perfectly well with Sphinx 5.3

marcinwrobel1986 · 2022-10-21T08:47:31Z

@JBludau regarding maintainability concerns, we can use this version of Sphinx and never upgrade. Python 3.10 end of life is set to 04 Oct 2026. IMHO the extension will not need any maintenance.

ajpowelsnl · 2024-07-30T15:53:48Z

Update

Main features enabling deprecation have already been merged (PR #189: Add cppkokkos extension to Sphinx #190 ), obviating the need for this PR

Marcin Wróbel added 2 commits September 16, 2022 11:22

kokkos#166: - modified Sphinx config (added created extension)

2e85b5a

- created cppkokkos based on kokkos - added view_test.rst in order to see how cppkokkos works

kokkos#166: - added support for cppkokkos regarding syntax highlighting

43a0f30

- removed `type` word before actual type in html

marcinwrobel1986 requested a review from fnrizzi September 22, 2022 06:14

marcinwrobel1986 self-assigned this Sep 22, 2022

marcinwrobel1986 marked this pull request as draft September 22, 2022 06:22

kokkos#166: - added support for deprecated version in cppkokkos domain

9b95aa2

- supports regex from kokkos releases - added support for changing colors in edit_button_handler - added deprecation type with version to view_test.rst

marcinwrobel1986 requested review from ajpowelsnl, crtrott, dalg24, JBludau and nmm0 September 26, 2022 06:56

ajpowelsnl approved these changes Oct 4, 2022

View reviewed changes

marcinwrobel1986 mentioned this pull request Oct 19, 2022

cppkokkos extension: status and next steps #187

Closed

JBludau reviewed Oct 20, 2022

View reviewed changes

marcinwrobel1986 removed their assignment Mar 3, 2023

ajpowelsnl closed this Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#166 Custom sphinx directive for deprecating a type alias #172

#166 Custom sphinx directive for deprecating a type alias #172

marcinwrobel1986 commented Sep 22, 2022 •

edited

Loading

marcinwrobel1986 commented Sep 26, 2022

JBludau commented Sep 26, 2022

nmm0 commented Sep 26, 2022

marcinwrobel1986 commented Sep 27, 2022

JBludau commented Sep 27, 2022

marcinwrobel1986 commented Sep 28, 2022 •

edited

Loading

JBludau commented Oct 3, 2022 •

edited

Loading

ajpowelsnl left a comment

ajpowelsnl Oct 4, 2022

marcinwrobel1986 Oct 19, 2022

ajpowelsnl commented Oct 4, 2022

marcinwrobel1986 commented Oct 19, 2022

JBludau commented Oct 19, 2022

JBludau left a comment

JBludau Oct 20, 2022

JBludau Oct 20, 2022

marcinwrobel1986 Oct 20, 2022

JBludau Oct 20, 2022

marcinwrobel1986 Oct 20, 2022 •

edited

Loading

JBludau Oct 21, 2022

marcinwrobel1986 Oct 22, 2022

marcinwrobel1986 commented Oct 21, 2022

marcinwrobel1986 commented Oct 21, 2022

marcinwrobel1986 commented Oct 21, 2022

ajpowelsnl commented Jul 30, 2024

#166 Custom sphinx directive for deprecating a type alias #172

#166 Custom sphinx directive for deprecating a type alias #172

Conversation

marcinwrobel1986 commented Sep 22, 2022 • edited Loading

THIS PR:

Test page look:

The *.rst syntax:

marcinwrobel1986 commented Sep 26, 2022

JBludau commented Sep 26, 2022

nmm0 commented Sep 26, 2022

marcinwrobel1986 commented Sep 27, 2022

JBludau commented Sep 27, 2022

marcinwrobel1986 commented Sep 28, 2022 • edited Loading

JBludau commented Oct 3, 2022 • edited Loading

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

ajpowelsnl commented Oct 4, 2022

marcinwrobel1986 commented Oct 19, 2022

JBludau commented Oct 19, 2022

JBludau 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

marcinwrobel1986 Oct 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcinwrobel1986 commented Oct 21, 2022

marcinwrobel1986 commented Oct 21, 2022

marcinwrobel1986 commented Oct 21, 2022

ajpowelsnl commented Jul 30, 2024

marcinwrobel1986 commented Sep 22, 2022 •

edited

Loading

The `*.rst` syntax:

marcinwrobel1986 commented Sep 28, 2022 •

edited

Loading

JBludau commented Oct 3, 2022 •

edited

Loading

marcinwrobel1986 Oct 20, 2022 •

edited

Loading