API-core-view-resize from md to rst #226
Conversation
pierrepebay
commented
Dec 14, 2022
•
pierrepebay
commented
- API/core/view/resize transition from .md to .rst
| before (.md) | after (.rst) |
|---|---|
 |
 |
 |
--- |
 |
 |
 |
 |
|
@pierrepebay one quick comment: to improve readability can you please ensure the arguments are all aligned? in the snipper below all args are aligned with Vieew and similarly for all others? for example in here: |
|
@fnrizzi not sure if this is the alignment changes you wanted, let me know if so. |
yes i think that is right, if that renders it correctly! |
|
avoided sphinx syntax because really messes up indentation of arguments, so it becomes hard to read. see #340 |
fnrizzi
force-pushed
the
120-API-core-view-resize
branch
from
d6ed66f to
6c839ec
Compare
fnrizzi
changed the title
| Description | ||
| ----------- | ||
|
|
||
| * .. code-block:: cpp |
There was a problem hiding this comment.
Please use :cpp:function:: (or rather our variant :cppkokkos:function::)
|
|
||
| * ``View<T, P...>::array_layout`` is either ``LayoutLeft`` or ``LayoutRight``. | ||
|
|
||
| * .. code-block:: cpp |
|
|
||
| * ``View<T, P...>::array_layout`` is either ``LayoutLeft` or `LayoutRight``. | ||
|
|
||
| * .. code-block:: cpp |
| * ``v``: existing view, can be a default constructed one. | ||
| * ``layout``: a layout instance containing the new dimensions. | ||
|
|
||
| * .. code-block:: cpp |
| * ``layout``: a layout instance containing the new dimensions. | ||
| * ``arg_prop``: View constructor property, e.g., ``Kokkos::WithoutInitializing``. | ||
|
|
||
| * .. code-block:: cpp |
|
@nmm0 see my comment "avoided sphinx syntax because really messes up indentation of arguments," |
|
Ack I saw Francesco's comment only after I submitted the review, please hold off on my suggested changes I don't want to give conflicting suggestions |
|
|
||
| Kokkos::resize(Kokkos::WithoutInitializing, v, 2, 3); | ||
|
|
||
| Resize a ``Kokkos::View`` with dynamic rank 2 to have dynamic extent 2 and 3 respectively preserving previous content. After this call, the new content is uninitialized. |
There was a problem hiding this comment.
Should the "Example" section have real, live working code that one could copy and paste? This is not an .md to .rst issue, but one relating to standardization. This issue does not have to be dealt with in the conversion exercise. I mention it because we probably want to move towards "live", working code, vs. pseudo code.
nmm0
dismissed
their stale review
Francesco pointed out the problem with my suggestions
|
@nmm0 can i merge this? |
I think it's fine for now though we may want to file an issue to update when the sphinx issue is resolved. Incidentally, how bad is the rendering currently? Is it worth losing the ability to cross reference these functions? |
|
@nmm0 here is an example (new rendering vs old for the same overload):
if you feel strongly that we should change, I am happy to do so |
Yeah that is pretty unreadable. Let's merge this now and we can merge on fixing up stuff when sphinx gets updated or we have a better solution (like trying utf8 ellipses or something like that :P) |
