Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
zhindes committed Mar 17, 2024
1 parent 12ba7a1 commit 5d93c2a
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 55 deletions.
6 changes: 0 additions & 6 deletions generated/nidaqmx/system/_collections/device_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,6 @@ def __getitem__(self, index):
return [_DeviceAlternateConstructor(name, self._interpreter) for name in self.device_names[index]]
elif isinstance(index, str):
device_names = unflatten_channel_string(index)
all_devices = self.device_names
# Validate the device names we were provided
for device in device_names:
if device not in all_devices:
raise KeyError(f'"{device}" is not a valid device name.')

if len(device_names) == 1:
return _DeviceAlternateConstructor(device_names[0], self._interpreter)
return [_DeviceAlternateConstructor(name, self._interpreter) for name in device_names]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,14 @@ def __getitem__(self, index):
return [_PhysicalChannelAlternateConstructor(channel, self._interpreter) for channel in self.channel_names[index]]
elif isinstance(index, str):
requested_channels = unflatten_channel_string(index)
all_channels = self.channel_names
# Validate the channel names we were provided
# Ensure the channel names are fully qualified. If the channel is invalid, the user will get errors from the
# channel objects on use.
channels_to_use = []
for channel in requested_channels:
if channel in all_channels:
if channel.startswith(self._name):
channels_to_use.append(channel)
else:
# The channel may have been unqualified, so we'll try to qualify it
qualified_channel = f'{self._name}/{channel}'
if qualified_channel in all_channels:
channels_to_use.append(qualified_channel)
else:
raise KeyError(f'"{channel}" is not a valid channel name.')
channels_to_use.append(f'{self._name}/{channel}')

if len(channels_to_use) == 1:
return _PhysicalChannelAlternateConstructor(channels_to_use[0], self._interpreter)
Expand Down
6 changes: 0 additions & 6 deletions src/handwritten/system/_collections/device_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,6 @@ def __getitem__(self, index):
return [_DeviceAlternateConstructor(name, self._interpreter) for name in self.device_names[index]]
elif isinstance(index, str):
device_names = unflatten_channel_string(index)
all_devices = self.device_names
# Validate the device names we were provided
for device in device_names:
if device not in all_devices:
raise KeyError(f'"{device}" is not a valid device name.')

if len(device_names) == 1:
return _DeviceAlternateConstructor(device_names[0], self._interpreter)
return [_DeviceAlternateConstructor(name, self._interpreter) for name in device_names]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,14 @@ def __getitem__(self, index):
return [_PhysicalChannelAlternateConstructor(channel, self._interpreter) for channel in self.channel_names[index]]
elif isinstance(index, str):
requested_channels = unflatten_channel_string(index)
all_channels = self.channel_names
# Validate the channel names we were provided
# Ensure the channel names are fully qualified. If the channel is invalid, the user will get errors from the
# channel objects on use.
channels_to_use = []
for channel in requested_channels:
if channel in all_channels:
if channel.startswith(self._name):
channels_to_use.append(channel)
else:
# The channel may have been unqualified, so we'll try to qualify it
qualified_channel = f'{self._name}/{channel}'
if qualified_channel in all_channels:
channels_to_use.append(qualified_channel)
else:
raise KeyError(f'"{channel}" is not a valid channel name.')
channels_to_use.append(f'{self._name}/{channel}')

if len(channels_to_use) == 1:
return _PhysicalChannelAlternateConstructor(channels_to_use[0], self._interpreter)
Expand Down
17 changes: 0 additions & 17 deletions tests/component/system/_collections/test_device_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,6 @@ def test___devices___getitem_str_list___shared_interpreter(system: System):
assert all(dev._interpreter is system._interpreter for dev in devices)


def test___devices___getitem_invalid_device_str___raises_error(system: System):
with pytest.raises(KeyError) as exc_info:
system.devices["foo"]

assert "foo" in exc_info.value.args[0]


def test___devices___getitem_invalid_device_str_list___raises_error(system: System):
if len(system.devices) == 0:
pytest.skip("This test requires at least one device.")

with pytest.raises(KeyError) as exc_info:
system.devices[f"{system.devices.device_names[0]},foo"]

assert "foo" in exc_info.value.args[0]


def test___devices___iter___forward_order(system: System):
devices = iter(system.devices)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,27 +204,30 @@ def test___physical_channels___getitem_mixed_str_list___name(


@pytest.mark.parametrize("collection_name", COLLECTION_NAMES)
def test___physical_channels___getitem_invalid_channel_str___raises_error(
def test___physical_channels___getitem_qualified_internal_channel_list___name(
collection_name: str, sim_6363_device: Device
):
physical_channels = getattr(sim_6363_device, collection_name)
internal_channel_names = [f"{sim_6363_device.name}/_ao{x}_vs_aognd" for x in range(4)]

with pytest.raises(KeyError) as exc_info:
physical_channels["foo"]
channels = physical_channels[",".join(internal_channel_names)]

assert "foo" in exc_info.value.args[0]
assert all(chan.name == name for chan, name in zip(channels, internal_channel_names))


@pytest.mark.parametrize("collection_name", COLLECTION_NAMES)
def test___physical_channels___getitem_invalid_channel_str_list___raises_error(
def test___physical_channels___getitem_unqualified_internal_channel_list___name(
collection_name: str, sim_6363_device: Device
):
physical_channels = getattr(sim_6363_device, collection_name)
internal_channel_names = ["_ao{x}_vs_aognd" for x in range(4)]

with pytest.raises(KeyError) as exc_info:
physical_channels[f"{physical_channels.channel_names[0]},foo"]
channels = physical_channels[",".join(internal_channel_names)]

assert "foo" in exc_info.value.args[0]
assert all(
chan.name == f"{sim_6363_device.name}/{name}"
for chan, name in zip(channels, internal_channel_names)
)


@pytest.mark.parametrize("collection_name", COLLECTION_NAMES)
Expand Down

0 comments on commit 5d93c2a

Please sign in to comment.