Add docs nitpick and reference fixes. (#2038)

* Add docs nitpick and reference fixes. * Add typing and extensions to nitpick ignore. * Apply suggestions from code review Co-authored-by: adai <[email protected]> --------- Co-authored-by: adai <[email protected]>
huawei-noah · May 27, 2023 · 40de4f0 · 40de4f0
1 parent afb84d0
commit 40de4f0
Show file tree

Hide file tree

Showing 30 changed files with 160 additions and 102 deletions.
diff --git a/.github/workflows/ci-format.yml b/.github/workflows/ci-format.yml
@@ -100,7 +100,7 @@ jobs:
           pip install wheel==0.38.4
           pip install .[camera_obs,doc,train,ray,envision,argoverse,opendrive,waymo]
           cd ${GITHUB_WORKSPACE}/docs
-          make html SPHINXOPTS="-W -T -E --keep-going -b spelling"
+          make html SPHINXOPTS="-W -T -E -n --keep-going -b spelling"
       - name: Check build output
         run: |
           cd $GITHUB_WORKSPACE

diff --git a/Makefile b/Makefile
@@ -154,7 +154,7 @@ format: gen-header
 
 .PHONY: docs
 docs:
-	cd docs && make clean html
+	cd docs && make clean html SPHINXOPTS="-W -T -n --keep-going -b spelling"
 
 .PHONY: wheel
 wheel: docs

diff --git a/docs/conf.py b/docs/conf.py
@@ -107,6 +107,45 @@
 # This pattern also affects html_static_path and html_extra_path.
 exclude_patterns = ["_build", "Thumbs.db", ".DS_Store"]
 
+# -- Options for nit-pick (-n) ----------------------------------------------------
+
+nitpick_ignore = {
+    ("py:class", "optional"),
+    ("py:class", "ellipsis"),
+    ("py:class", "function"),
+    # Most of these dynamic type ignores would go away in python>=3.10
+    # See for more context: https:github.com/sphinx-doc/sphinx/issues/10090
+    ("py:class", "T"),
+    ("py:class", "Score"),
+    ("py:class", "Done"),
+    ("py:class", "CostFuncs"),
+    ("py:class", "ActType"),
+    ("py:class", "ObsType"),
+    ("py:class", "smarts.env.gymnasium.wrappers.metric.utils.T"),
+}
+nitpick_ignore_regex = {
+    (r"py:.*", r"av2\..*"),
+    (r"py:.*", r"google\.protobuf\..*"),
+    (r"py:.*", r"grpc\..*"),
+    (r"py:.*", r"gym\..*"),
+    (r"py:.*", r"gymnasium\..*"),
+    (r"py:.*", r"logging\..*"),
+    (r"py:.*", r"multiprocessing\..*"),
+    (r"py:.*", r"np\..*"),
+    (r"py:.*", r"numpy\..*"),
+    (r"py:.*", r"opendrive2lanelet\..*"),
+    (r"py:.*", r"panda3d\..*"),
+    (r"py:.*", r"pathlib\..*"),
+    (r"py:.*", r"pybullet(_utils)?\..*"),
+    (r"py:.*", r"re\..*"),
+    (r"py:.*", r"shapely\..*"),
+    (r"py:.*", r"sumo(lib)?\..*"),
+    (r"py:.*", r"tornado\..*"),
+    (r"py:.*", r"traci\..*"),
+    (r"py:.*", r"typing(_extensions)?\..*"),
+    (r"py:.*", r"configparser\..*"),
+}
+
 # -- Options for spelling ----------------------------------------------------
 spelling_exclude_patterns = ["ignored_*", "**/*_pb2*"]
 spelling_ignore_pypi_package_names = True

diff --git a/docs/sim/obs_action_reward.rst b/docs/sim/obs_action_reward.rst
@@ -100,5 +100,5 @@ An agent can be configured to emit any one of the following action types from :c
 .. tip::
 
     Depending on the agent's policy, :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.ActuatorDynamic` action type might 
-    allow the agent to learn faster than :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.Continous` action type because 
+    allow the agent to learn faster than :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.Continuous` action type because 
     learning to correct steering could be simpler than learning a mapping to all the absolute steering angle values. 
diff --git a/smarts/benchmark/__init__.py b/smarts/benchmark/__init__.py
@@ -20,12 +20,14 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 import importlib
+import pathlib
 import subprocess
 import sys
-from pathlib import Path
 from typing import Any, Dict, List, Optional
 
-BENCHMARK_LISTING_FILE = Path(__file__).parent.absolute() / "benchmark_listing.yaml"
+BENCHMARK_LISTING_FILE = (
+    pathlib.Path(__file__).parent.absolute() / "benchmark_listing.yaml"
+)
 
 
 def auto_install_requirements(benchmark_spec: Dict[str, Any]):
@@ -71,7 +73,7 @@ def run_benchmark(
     benchmark_name: str,
     benchmark_version: Optional[float],
     agent_locator: str,
-    benchmark_listing: Path,
+    benchmark_listing: pathlib.Path,
     debug_log: bool = False,
     auto_install: bool = False,
 ):
@@ -82,7 +84,7 @@ def run_benchmark(
         benchmark_name(str): The name of the benchmark to run.
         benchmark_version(float|None): The version of the benchmark.
         agent_locator(str): Locator string for the registered agent.
-        benchmark_listing(Path): A configuration file that lists benchmark metadata and must list
+        benchmark_listing(pathlib.Path): A configuration file that lists benchmark metadata and must list
             the target benchmark.
         debug_log: Debug to `stdout`.
     """
@@ -117,4 +119,4 @@ def list_benchmarks(benchmark_listing):
     """Lists details of the currently available benchmarks."""
     from smarts.core.utils.resources import load_yaml_config_with_substitution
 
-    return load_yaml_config_with_substitution(Path(benchmark_listing))
+    return load_yaml_config_with_substitution(pathlib.Path(benchmark_listing))
diff --git a/smarts/benchmark/entrypoints/benchmark_runner_v0.py b/smarts/benchmark/entrypoints/benchmark_runner_v0.py
@@ -212,9 +212,9 @@ def benchmark_from_configs(benchmark_config, agent_locator, debug_log=False):
     """Runs a benchmark given the following.
 
     Args:
-        benchmark_config(file path): The file path to the benchmark configuration.
-        agent_locator(str): Locator string for the registered agent.
-        debug_log(bool): Whether the benchmark should log to `stdout`.
+        benchmark_config (str): The file path to the benchmark configuration.
+        agent_locator (str): Locator string for the registered agent.
+        debug_log (bool): Whether the benchmark should log to `stdout`.
     """
     benchmark_args = load_config(benchmark_config)
 

diff --git a/smarts/core/bezier_motion_planner.py b/smarts/core/bezier_motion_planner.py
@@ -38,14 +38,14 @@ def trajectory_batched(self, current_poses, target_poses_at_t, n, dt) -> np.ndar
         """Generate a batch of trajectories
 
         Args:
-            current_poses: np.array([[x, y, heading]])
-            target_poses_at_t: np.array([[x, y, heading, seconds_into_future]]
+            current_poses: ``np.array([[x, y, heading]])``
+            target_poses_at_t: ``np.array([[x, y, heading, seconds_into_future]]``
                     pose we would like to have this many seconds into the future
             n: number of trajectory points to return
             dt: time delta between trajectory points
         Returns:
-            Stacked np.array of the form:
-                np.array([[x], [y], [heading], [desired_speed]])
+            A stacked ``np.array`` of the form
+                ``np.array([[x], [y], [heading], [desired_speed]])``
         """
         assert len(current_poses) == len(target_poses_at_t)
         # vectorized cubic bezier computation

diff --git a/smarts/core/chassis.py b/smarts/core/chassis.py
@@ -143,7 +143,7 @@ def pose(self) -> Pose:
 
     @property
     def steering(self) -> float:
-        """The steering value of the chassis in radians [-math.pi:math.pi]."""
+        """The steering value of the chassis in radians [-math.pi, math.pi]."""
         raise NotImplementedError
 
     @property

diff --git a/smarts/core/configuration.py b/smarts/core/configuration.py
@@ -23,7 +23,7 @@
 import configparser
 import functools
 import os
-from pathlib import Path
+import pathlib
 from typing import Any, Callable, Optional, Union
 
 _UNSET = object()
@@ -49,15 +49,15 @@ class Config:
     """A configuration utility that handles configuration from file and environment variable.
 
     Args:
-        config_file (Union[str, Path]): The path to the configuration file.
+        config_file (Union[str, pathlib.Path]): The path to the configuration file.
         environment_prefix (str, optional): The prefix given to the environment variables. Defaults to "".
 
     Raises:
         FileNotFoundError: If the configuration file cannot be found at the given file location.
     """
 
     def __init__(
-        self, config_file: Union[str, Path], environment_prefix: str = ""
+        self, config_file: Union[str, pathlib.Path], environment_prefix: str = ""
     ) -> None:
         self._config = configparser.ConfigParser(
             interpolation=configparser.ExtendedInterpolation()
@@ -66,7 +66,7 @@ def __init__(
         self._environment_variable_format_string = self._environment_prefix + "_{}_{}"
 
         if isinstance(config_file, str):
-            config_file = Path(config_file)
+            config_file = pathlib.Path(config_file)
         config_file = config_file.resolve()
         if not config_file.is_file():
             raise FileNotFoundError(f"Configuration file not found at {config_file}")

diff --git a/smarts/core/road_map.py b/smarts/core/road_map.py
@@ -362,18 +362,28 @@ def lanes_in_same_direction(self) -> List[RoadMap.Lane]:
 
         @property
         def lane_to_left(self) -> Tuple[RoadMap.Lane, bool]:
-            """Note: left is defined as 90 degrees clockwise relative to the lane heading.
-            (I.e., positive `t` in the RefLine coordinate system.)
-            Second result is True if lane is in the same direction as this one
-            In junctions, diverging lanes should not be included."""
+            """Get the parallel lane to the left side of the direction this current lane.
+
+            .. note::
+
+                Left is defined as 90 degrees clockwise relative to the lane heading.
+                (I.e., positive `t` in the RefLine coordinate system.)
+                Second result is True if lane is in the same direction as this one
+                In junctions, diverging lanes should not be included.
+            """
             raise NotImplementedError()
 
         @property
         def lane_to_right(self) -> Tuple[RoadMap.Lane, bool]:
-            """Note: right is defined as 90 degrees counter-clockwise relative to the lane heading.
-            (I.e., negative `t` in the RefLine coordinate system.)
-            Second result is True if lane is in the same direction as this one.
-            In junctions, diverging lanes should not be included."""
+            """Get the parallel lane to the right side of the direction this current lane.
+
+            .. note::
+
+                Right is defined as 90 degrees counter-clockwise relative to the lane heading.
+                (I.e., negative `t` in the RefLine coordinate system.)
+                Second result is True if lane is in the same direction as this one.
+                In junctions, diverging lanes should not be included.
+            """
             raise NotImplementedError()
 
         @property

diff --git a/smarts/core/scenario.py b/smarts/core/scenario.py
@@ -631,7 +631,7 @@ def create_dynamic_traffic_history_mission(
             positional_radius:
                 The goal radius for the positional goal.
         Returns:
-            (positional_mission, traverse_mission): A positional mission that follows the initial
+            (smarts.core.plan.Mission, smarts.core.plan.Mission): A positional mission that follows the initial
              original vehicle's travel as well as a traverse style mission which is done when the
              vehicle leaves the map.
         """

diff --git a/smarts/core/smarts.py b/smarts/core/smarts.py
@@ -98,7 +98,7 @@ class SMARTS(ProviderManager):
     Args:
         agent_interfaces (Dict[str, AgentInterface]): The interfaces providing SMARTS with the understanding of what features each agent needs.
         traffic_sims (Optional[List[TrafficProvider]], optional): An optional list of traffic simulators for providing non-agent traffic. Defaults to None.
-        envision (Optional[EnvisionClient], optional): An envision client for connecting to an envision visualization server. Defaults to None.
+        envision (Optional[envision.client.Client], optional): An envision client for connecting to an envision visualization server. Defaults to None.
         visdom (Union[bool, Any], optional): Deprecated. Use SMARTS_VISDOM_ENABLED. A visdom client for connecting to a visdom visualization server.
         fixed_timestep_sec (Optional[float], optional): The fixed timestep that will be default if time is not otherwise specified at step. Defaults to 0.1.
         reset_agents_only (bool, optional): When specified the simulation will continue use of the current scenario. Defaults to False.
@@ -244,7 +244,7 @@ def step(
                 Overrides the simulation step length. Progress simulation time by the given amount.
                 Note the time_delta_since_last_step param is in (nominal) seconds.
         Returns:
-            Tuple[observations, rewards, dones, infos]: The simulation step return.
+            The simulation step return as (observations, rewards, dones, infos).
         """
         if not self._is_setup:
             raise SMARTSNotSetupError("Must call reset() or setup() before stepping.")
@@ -428,15 +428,19 @@ def reset(
          are no agents in the simulation.
 
         Args:
-            scenario(smarts.core.scenario.Scenario): The scenario to reset the simulation with.
-            start_time(float):
+            scenario (smarts.core.scenario.Scenario): The scenario to reset the simulation with.
+            start_time (float):
                 The initial amount of simulation time to skip. This has implications on all time
-                dependent systems. NOTE: SMARTS simulates a step and then updates vehicle control.
-                If you want a vehicle to enter at exactly `0.3` with a step of `0.1` it means the
-                simulation should start at `start_time==0.2`.
+                dependent systems.
+
+                .. note::
+
+                    SMARTS simulates a step and then updates vehicle control.
+                    If you want a vehicle to enter at exactly ``0.3`` with a step of ``0.1`` it means the
+                    simulation should start at ``start_time==0.2``.
 
         Returns:
-            Agent observations. This observation is as follows:
+            Dict[str, Observation]. This observation is as follows...
                 - If no agents: the initial simulation observation at `start_time`
                 - If agents: the first step of the simulation with an agent observation
         """

diff --git a/smarts/core/trap_manager.py b/smarts/core/trap_manager.py
@@ -116,14 +116,16 @@ def add_trap_for_agent(
         :param agent_id: The agent to associate to this trap.
         :type agent_id: str
         :param mission: The mission to assign to the agent and vehicle.
-        :type mission: class: Mission
+        :type mission: smarts.core.plan.Mission
         :param road_map: The road map to provide information to about the map.
-        :type road_map: class: RoadMap
+        :type road_map: RoadMap
         :param sim_time: The current simulator time.
         :type sim_time: float
         :param reject_expired: If traps should be ignored if their patience would already be
             expired on creation
-        :type reject_expired: Tuple[bool, bool] If the trap was added and if the trap is already expired.
+        :type reject_expired: bool
+        :return: If the trap was added and if the trap is already expired.
+        :rtype: Tuple[bool, bool]
         """
         if mission is None:
             mission = Mission.random_endless_mission(road_map)

diff --git a/smarts/core/utils/math.py b/smarts/core/utils/math.py
@@ -436,7 +436,7 @@ def position_to_ego_frame(position, ego_position, ego_heading):
         ego_position: Ego vehicle [x,y,z]
         ego_heading: Ego vehicle heading in radians
     Returns:
-        new_pose: The pose [x,y,z] in egocentric view
+        list: The position [x,y,z] in egocentric view
     """
     transform_matrix = _gen_ego_frame_matrix(ego_heading)
     ego_rel_position = np.asarray(position) - np.asarray(ego_position)
@@ -453,7 +453,7 @@ def world_position_from_ego_frame(position, ego_world_position, ego_world_headin
         ego_world_position: Ego vehicle [x,y,z]
         ego_world_heading: Ego vehicle heading in radians
     Returns:
-        new_pose: The pose [x,y,z] in world frame
+        list: The position [x,y,z] in world frame
     """
     transform_matrix = _gen_ego_frame_matrix(ego_world_heading)
     transform_matrix = np.linalg.inv(transform_matrix)

diff --git a/smarts/env/gymnasium/driving_smarts_2022_env.py b/smarts/env/gymnasium/driving_smarts_2022_env.py
@@ -69,7 +69,7 @@ def driving_smarts_2022_env(
         Action space for each agent is configured through its `AgentInterface`.
         The action space could be either of the following.
 
-        (i) :attr:`~smarts.core.controllers.ActionSpaceType.RelativeTargetPose`
+        (i) :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.RelativeTargetPose`
 
            +------------------------------------+-------------+-------+
            | Action                             | Values      | Units |
@@ -81,7 +81,7 @@ def driving_smarts_2022_env(
            | Δheading                           | [-π, π]     | rad   |
            +------------------------------------+-------------+-------+
 
-        (ii) :attr:`~smarts.core.controllers.ActionSpaceType.TargetPose`
+        (ii) :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.TargetPose`
 
             +------------------------------------+---------------+-------+
             | Action                             | Values        | Units |

diff --git a/smarts/env/gymnasium/driving_smarts_2023_env.py b/smarts/env/gymnasium/driving_smarts_2023_env.py
@@ -68,8 +68,8 @@ def driving_smarts_2023_env(
         a sample formatted observation data structure.
 
     Action space for each agent:
-        Action space for an ego can be either :attr:`~smarts.core.controllers.ActionSpaceType.Continuous`
-        or :attr:`~smarts.core.controllers.ActionSpaceType.RelativeTargetPose`. User should choose
+        Action space for an ego can be either :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.Continuous`
+        or :attr:`~smarts.core.controllers.action_space_type.ActionSpaceType.RelativeTargetPose`. User should choose
         one of the action spaces and specify the chosen action space through the ego's agent interface.
 
     Agent interface: