Skip to content

Commit

Permalink
Issue #64: Use exceptions instead of assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
Mark2000 committed Sep 18, 2023
1 parent 9b5beae commit c75b6a9
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 19 deletions.
5 changes: 4 additions & 1 deletion bsk_rl/envs/general_satellite_tasking/gym_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ class SingleSatelliteTasking(GeneralSatelliteTasking):

def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
assert len(self.satellites) == 1
if not len(self.satellites) == 1:
raise ValueError(
"SingleSatelliteTasking must be initialized with a single satellite."
)

@property
def action_space(self) -> spaces.Discrete:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ def __init__(self, satellites: list["Satellite"]) -> None:
sight"""
super().__init__(satellites)
for satellite in self.satellites:
assert issubclass(satellite.dyn_type, LOSCommDynModel)
if not issubclass(satellite.dyn_type, LOSCommDynModel):
raise TypeError(
f"Satellite dynamics type {satellite.dyn_type} must be a subclass "
+ "of LOSCommDynModel to use LOSCommunication"
)

def reset(self) -> None:
super().reset()
Expand Down
3 changes: 2 additions & 1 deletion bsk_rl/envs/general_satellite_tasking/scenario/satellites.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def default_sat_args(cls, **kwargs) -> dict[str, Any]:
)

for k, v in kwargs.items():
assert k in defaults, f"{k} not a parameter"
if k not in defaults:
raise KeyError(f"{k} not a valid key for sat_args")
defaults[k] = v
return defaults

Expand Down
21 changes: 14 additions & 7 deletions bsk_rl/envs/general_satellite_tasking/simulation/dynamics.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from abc import ABC, abstractmethod
from typing import TYPE_CHECKING, Iterable, Optional
from warnings import warn

if TYPE_CHECKING: # pragma: no cover
from bsk_rl.envs.general_satellite_tasking.types import (
Expand Down Expand Up @@ -64,12 +65,13 @@ def __init__(
priority: Model priority.
"""
self.satellite = satellite
assert all(
[
issubclass(type(self.simulator.environment), required)
for required in self.requires_env
]
)

for required in self.requires_env:
if not issubclass(type(self.simulator.environment), required):
raise TypeError(
f"{self.simulator.environment} must be a subclass of {required} to "
+ f"use dynamics model of type {self.__class__}"
)

dyn_proc_name = "DynamicsProcess" + self.satellite.id
self.dyn_proc = self.simulator.CreateNewProcess(dyn_proc_name, priority)
Expand Down Expand Up @@ -630,7 +632,11 @@ def _set_transmitter(
transmitterNumBuffers: Number of transmitter buffers
priority: Model priority.
"""
assert transmitterBaudRate <= 0
if transmitterBaudRate > 0:
warn(
"Positive transmitterBaudRate will lead to increased data in buffer "
+ "on downlink"
)
self.transmitter = spaceToGroundTransmitter.SpaceToGroundTransmitter()
self.transmitter.ModelTag = "transmitter" + self.satellite.id
self.transmitter.nodeBaudRate = transmitterBaudRate # baud
Expand Down Expand Up @@ -801,3 +807,4 @@ def _set_ground_station_locations(self) -> None:

class FullFeaturedDynModel(GroundStationDynModel, LOSCommDynModel):
pass
pass
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def default_env_args(cls, **kwargs) -> dict[str, Any]:
"""Compile default argments for the environment model"""
defaults = collect_default_args(cls)
for k, v in kwargs.items():
assert k in defaults, f"{k} not a parameter"
if k not in defaults:
raise KeyError(f"{k} not a valid key for env_args")
defaults[k] = v
return defaults

Expand Down
14 changes: 9 additions & 5 deletions bsk_rl/envs/general_satellite_tasking/simulation/fsw.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@ def __init__(
"""

self.satellite = satellite
assert all(
[issubclass(satellite.dyn_type, required) for required in self.requires_dyn]
)

for required in self.requires_dyn:
if not issubclass(satellite.dyn_type, required):
raise TypeError(
f"{satellite.dyn_type} must be a subclass of {required} to "
+ f"use FSW model of type {self.__class__}"
)

fsw_proc_name = "FSWProcess" + self.satellite.id
self.fsw_proc = self.simulator.CreateNewProcess(fsw_proc_name, priority)
Expand Down Expand Up @@ -638,7 +642,7 @@ def _set_instrument_controller(
self,
imageAttErrorRequirement: float,
imageRateErrorRequirement: float,
**kwargs
**kwargs,
) -> None:
"""Defines the instrument controller parameters.
Expand Down Expand Up @@ -744,7 +748,7 @@ def _set_mrp_steering_rwa(
omega_max: float,
servo_Ki: float,
servo_P: float,
**kwargs
**kwargs,
) -> None:
"""Defines the control properties.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_default_sat_args_overwrote(self, overwrite, error):
"c": 4,
}
else:
with pytest.raises(AssertionError):
with pytest.raises(KeyError):
sats.Satellite.default_sat_args(**overwrite)

def test_init_default(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def test_default_sat_args_overwrote(self, mock_collect, overwrite, error):
"c": 4,
}
else:
with pytest.raises(AssertionError):
with pytest.raises(KeyError):
EnvironmentModel.default_env_args(**overwrite)

@patch.multiple(EnvironmentModel, __abstractmethods__=set())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def test_init(self):
assert env.satellite == mock_sat

def test_init_multisat(self):
with pytest.raises(AssertionError):
with pytest.raises(ValueError):
SingleSatelliteTasking(
satellites=[MagicMock(), MagicMock()],
env_type=MagicMock(),
Expand Down

0 comments on commit c75b6a9

Please sign in to comment.