Improves contribution guidelines for IsaacLab (isaac-sim#3403)

# Description

I realized there were some comments I often have to repeat in my
reviewing process. I tried to add some of them into the code style page
to directly point developers to it.

## Type of change

- This change requires a documentation update

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

---------

Signed-off-by: Mayank Mittal <[email protected]>
Co-authored-by: James Tigue <[email protected]>

Author: 2 people

.github/PULL_REQUEST_TEMPLATE.md

@@ -4,6 +4,8 @@
 Thank you for your interest in sending a pull request. Please make sure to check the contribution guidelines.
 
 Link: https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html
+
+💡 Please try to keep PRs small and focused. Large PRs are harder to review and merge.
 -->
 
 Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
@@ -21,8 +23,8 @@ is demanded by more than one party. -->
 
 - Bug fix (non-breaking change which fixes an issue)
 - New feature (non-breaking change which adds functionality)
-- Breaking change (fix or feature that would cause existing functionality to not work as expected)
-- This change requires a documentation update
+- Breaking change (existing functionality will not work without user modification)
+- Documentation update
 
 ## Screenshots
 
@@ -40,6 +42,7 @@ To upload images to a PR -- simply drag and drop an image while in edit mode and
 
 ## Checklist
 
+- [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
 - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format`
 - [ ] I have made corresponding changes to the documentation
 - [ ] My changes generate no new warnings

docs/conf.py

@@ -122,7 +122,7 @@
     "python": ("https://docs.python.org/3", None),
     "numpy": ("https://numpy.org/doc/stable/", None),
     "trimesh": ("https://trimesh.org/", None),
-    "torch": ("https://pytorch.org/docs/stable/", None),
+    "torch": ("https://docs.pytorch.org/docs/stable", None),
     "isaacsim": ("https://docs.isaacsim.omniverse.nvidia.com/5.0.0/py/", None),
     "gymnasium": ("https://gymnasium.farama.org/", None),
     "warp": ("https://nvidia.github.io/warp/", None),
@@ -162,6 +162,7 @@
     "isaacsim.core.api",
     "isaacsim.core.cloner",
     "isaacsim.core.version",
+    "isaacsim.core.utils",
     "isaacsim.robot_motion.motion_generation",
     "isaacsim.gui.components",
     "isaacsim.asset.importer.urdf",

docs/source/overview/environments.rst

@@ -190,12 +190,10 @@ for the lift-cube environment:
 .. |galbot_stack-link| replace:: `Isaac-Stack-Cube-Galbot-Left-Arm-Gripper-RmpFlow-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/stack/config/galbot/stack_rmp_rel_env_cfg.py>`__
 .. |kuka-allegro-lift-link| replace:: `Isaac-Dexsuite-Kuka-Allegro-Lift-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/dexsuite/dexsuite_kuka_allegro_env_cfg.py>`__
 .. |kuka-allegro-reorient-link| replace:: `Isaac-Dexsuite-Kuka-Allegro-Reorient-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/dexsuite/dexsuite_kuka_allegro_env_cfg.py>`__
-
 .. |cube-shadow-link| replace:: `Isaac-Repose-Cube-Shadow-Direct-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/direct/shadow_hand/shadow_hand_env_cfg.py>`__
 .. |cube-shadow-ff-link| replace:: `Isaac-Repose-Cube-Shadow-OpenAI-FF-Direct-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/direct/shadow_hand/shadow_hand_env_cfg.py>`__
 .. |cube-shadow-lstm-link| replace:: `Isaac-Repose-Cube-Shadow-OpenAI-LSTM-Direct-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/direct/shadow_hand/shadow_hand_env_cfg.py>`__
 .. |cube-shadow-vis-link| replace:: `Isaac-Repose-Cube-Shadow-Vision-Direct-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/direct/shadow_hand/shadow_hand_vision_env.py>`__
-
 .. |agibot_place_mug-link| replace:: `Isaac-Place-Mug-Agibot-Left-Arm-RmpFlow-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/place/config/agibot/place_upright_mug_rmp_rel_env_cfg.py>`__
 .. |agibot_place_toy-link| replace:: `Isaac-Place-Toy2Box-Agibot-Right-Arm-RmpFlow-v0 <https://github.com/isaac-sim/IsaacLab/blob/main/source/isaaclab_tasks/isaaclab_tasks/manager_based/manipulation/place/config/agibot/place_toy2box_rmp_rel_env_cfg.py>`__
 

docs/source/refs/contributing.rst

@@ -54,6 +54,9 @@ Please ensure that your code is well-formatted, documented and passes all the te
    Large pull requests are difficult to review and may take a long time to merge.
 
 
+More details on the code style and testing can be found in the `Coding Style`_ and `Unit Testing`_ sections.
+
+
 Contributing Documentation
 --------------------------
 
@@ -237,6 +240,62 @@ For documentation, we adopt the `Google Style Guide <https://sphinxcontrib-napol
 for docstrings. We use `Sphinx <https://www.sphinx-doc.org/en/master/>`__ for generating the documentation.
 Please make sure that your code is well-documented and follows the guidelines.
 
+Code Structure
+^^^^^^^^^^^^^^
+
+We follow a specific structure for the codebase. This helps in maintaining the codebase and makes it easier to
+understand.
+
+In a Python file, we follow the following structure:
+
+.. code:: python
+
+   # Imports: These are sorted by the pre-commit hooks.
+   # Constants
+   # Functions (public)
+   # Classes (public)
+   # _Functions (private)
+   # _Classes (private)
+
+Imports are sorted by the pre-commit hooks. Unless there is a good reason to do otherwise, please do not
+import the modules inside functions or classes. To deal with circular imports, we use the
+:obj:`typing.TYPE_CHECKING` variable. Please refer to the `Circular Imports`_ section for more details.
+
+Python does not have a concept of private and public classes and functions. However, we follow the
+convention of prefixing the private functions and classes with an underscore.
+The public functions and classes are the ones that are intended to be used by the users. The private
+functions and classes are the ones that are intended to be used internally in that file.
+Irrespective of the public or private nature of the functions and classes, we follow the Style Guide
+for the code and make sure that the code and documentation are consistent.
+
+Similarly, within Python classes, we follow the following structure:
+
+.. code:: python
+
+   # Constants
+   # Class variables (public or private): Must have the type hint ClassVar[type]
+   # Dunder methods: __init__, __del__
+   # Representation: __repr__, __str__
+   # Properties: @property
+   # Instance methods (public)
+   # Class methods (public)
+   # Static methods (public)
+   # _Instance methods (private)
+   # _Class methods (private)
+   # _Static methods (private)
+
+The rule of thumb is that the functions within the classes are ordered in the way a user would
+expect to use them. For instance, if the class contains the method :meth:`initialize`, :meth:`reset`,
+:meth:`update`, and :meth:`close`, then they should be listed in the order of their usage.
+The same applies for private functions in the class. Their order is based on the order of call inside the
+class.
+
+.. dropdown:: Code skeleton
+   :icon: code
+
+   .. literalinclude:: snippets/code_skeleton.py
+      :language: python
+
 Circular Imports
 ^^^^^^^^^^^^^^^^
 
@@ -414,15 +473,47 @@ We summarize the key points below:
 
 
 Unit Testing
-^^^^^^^^^^^^
+------------
 
 We use `pytest <https://docs.pytest.org>`__ for unit testing.
 Good tests not only cover the basic functionality of the code but also the edge cases.
 They should be able to catch regressions and ensure that the code is working as expected.
 Please make sure that you add tests for your changes.
 
+.. tab-set::
+   :sync-group: os
+
+   .. tab-item:: :icon:`fa-brands fa-linux` Linux
+      :sync: linux
+
+      .. code-block:: bash
+
+         # Run all tests
+         ./isaaclab.sh --test  # or "./isaaclab.sh -t"
+
+         # Run all tests in a particular file
+         ./isaaclab.sh -p -m pytest source/isaaclab/test/deps/test_torch.py
+
+         # Run a particular test
+         ./isaaclab.sh -p -m pytest source/isaaclab/test/deps/test_torch.py::test_array_slicing
+
+   .. tab-item:: :icon:`fa-brands fa-windows` Windows
+      :sync: windows
+
+      .. code-block:: bash
+
+         # Run all tests
+         isaaclab.bat --test  # or "isaaclab.bat -t"
+
+         # Run all tests in a particular file
+         isaaclab.bat -p -m pytest source/isaaclab/test/deps/test_torch.py
+
+         # Run a particular test
+         isaaclab.bat -p -m pytest source/isaaclab/test/deps/test_torch.py::test_array_slicing
+
+
 Tools
-^^^^^
+-----
 
 We use the following tools for maintaining code quality:
 
@@ -435,6 +526,19 @@ Please check `here <https://pre-commit.com/#install>`__ for instructions
 to set these up. To run over the entire repository, please execute the
 following command in the terminal:
 
-.. code:: bash
+.. tab-set::
+   :sync-group: os
+
+   .. tab-item:: :icon:`fa-brands fa-linux` Linux
+      :sync: linux
+
+      .. code-block:: bash
+
+         ./isaaclab.sh --format  # or "./isaaclab.sh -f"
+
+   .. tab-item:: :icon:`fa-brands fa-windows` Windows
+      :sync: windows
+
+      .. code-block:: bash
 
-   ./isaaclab.sh --format  # or "./isaaclab.sh -f"
+         isaaclab.bat --format  # or "isaaclab.bat -f"

docs/source/refs/snippets/code_skeleton.py

@@ -0,0 +1,155 @@
+# Copyright (c) 2022-2025, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md).
+# All rights reserved.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+import os
+import sys
+from typing import ClassVar
+
+
+DEFAULT_TIMEOUT: int = 30
+"""Default timeout for the task."""
+
+_MAX_RETRIES: int = 3  # private constant (note the underscore)
+"""Maximum number of retries for the task."""
+
+
+def run_task(task_name: str):
+    """Run a task by name.
+
+    Args:
+        task_name: The name of the task to run.
+    """
+    print(f"Running task: {task_name}")
+
+
+class TaskRunner:
+    """Runs and manages tasks."""
+
+    DEFAULT_NAME: ClassVar[str] = "runner"
+    """Default name for the runner."""
+
+    _registry: ClassVar[dict] = {}
+    """Registry of runners."""
+
+    def __init__(self, name: str):
+        """Initialize the runner.
+
+        Args:
+            name: The name of the runner.
+        """
+        self.name = name
+        self._tasks = []  # private instance variable
+
+    def __del__(self):
+        """Clean up the runner."""
+        print(f"Cleaning up {self.name}")
+
+    def __repr__(self) -> str:
+        return f"TaskRunner(name={self.name!r})"
+
+    def __str__(self) -> str:
+        return f"TaskRunner: {self.name}"
+
+    """
+    Properties.
+    """
+
+    @property
+    def task_count(self) -> int:
+        return len(self._tasks)
+
+    """
+    Operations.
+    """
+
+    def initialize(self):
+        """Initialize the runner."""
+        print("Initializing runner...")
+
+    def update(self, task: str):
+        """Update the runner with a new task.
+
+        Args:
+            task: The task to add.
+        """
+        self._tasks.append(task)
+        print(f"Added task: {task}")
+
+    def close(self):
+        """Close the runner."""
+        print("Closing runner...")
+
+    """
+    Operations: Registration.
+    """
+
+    @classmethod
+    def register(cls, name: str, runner: "TaskRunner"):
+        """Register a runner.
+
+        Args:
+            name: The name of the runner.
+            runner: The runner to register.
+        """
+        if name in cls._registry:
+            _log_error(f"Runner {name} already registered. Skipping registration.")
+            return
+        cls._registry[name] = runner
+
+    @staticmethod
+    def validate_task(task: str) -> bool:
+        """Validate a task.
+
+        Args:
+            task: The task to validate.
+
+        Returns:
+            True if the task is valid, False otherwise.
+        """
+        return bool(task and task.strip())
+
+    """
+    Internal operations.
+    """
+
+    def _reset(self):
+        """Reset the runner."""
+        self._tasks.clear()
+
+    @classmethod
+    def _get_registry(cls) -> dict:
+        """Get the registry."""
+        return cls._registry
+
+    @staticmethod
+    def _internal_helper():
+        """Internal helper."""
+        print("Internal helper called.")
+
+
+"""
+Helper operations.
+"""
+
+
+def _log_error(message: str):
+    """Internal helper to log errors.
+
+    Args:
+        message: The message to log.
+    """
+    print(f"[ERROR] {message}")
+
+
+class _TaskHelper:
+    """Private utility class for internal task logic."""
+
+    def compute(self) -> int:
+        """Compute the result.
+
+        Returns:
+            The result of the computation.
+        """
+        return 42