Empty list not allowed as default value for PARAMETER_STRING_ARRAY

[NikolasE]

Edit: Sorry, Error message was produced when using the patch proposed in https://github.com/ros2/rclpy/pull/887/files

Standard behavior is to ignore the parameterDescriptor and create a byte-array (https://github.com/ros2/rclpy/issues/912#issuecomment-1069681520)

So it's essentially a duplicate of https://github.com/ros2/rclpy/issues/879 :(

It seems not possible to use an empty list as default value for a string-array:

  self.declare_parameter("str_list", [], ParameterDescriptor(
                  type=ParameterType.PARAMETER_STRING_ARRAY,
                  description="optionally some strings",
              ))

It seems that an empty list is assumed to be a byte_array:

('Invalid parameter value ([]) for parameter. Reason: Passed value is of type 5, but ParameterDescriptor declared type 9'

Do we need an additional check for empty lists?

[fujitatomoya]

could you provide more information such as what version and distribution you use? i think new issue templates provide relevant categories to fill in.

[fujitatomoya]

('Invalid parameter value ([]) for parameter. Reason: Passed value is of type 5, but ParameterDescriptor declared type 9'

i do not see this error with mainline https://github.com/ros2/ros2/commit/9597c36591c643a1ff6d87964ddd3d999f191baf. but actual value is honored than descriptor, i think that the question here is.

    node.declare_parameter("str_list", [],
        ParameterDescriptor(
            type=ParameterType.PARAMETER_STRING_ARRAY,
            description="optionally some strings",
        )
    )

then, we get

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param describe /rclpy_912 str_list
Parameter name: str_list
  Type: byte array
  Description: optionally some strings
  Constraints:

this can be fixed with, https://github.com/fujitatomoya/ros2_test_prover/blob/08b1b3d391c8297a6908a578ade059fd16c20821/prover_rclpy/src/rclpy_912.py#L23

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param describe /rclpy_912 str_list
Parameter name: str_list
  Type: string array
  Description: optionally some strings
  Constraints:

i think current behavior is okay which is actual value prevails against the descriptor.

[NikolasE]

A list that contains and empty string is definitely not the same as an empty list. I use it to define some tf-frames and would either have to implement a special filter for the empty string or get the standard error message that it's not a valid frame.

And I think it's a very surprising behavior to get a byte-array-parameter after you explicitly created a str-list-parameter. The user also has no way to guess which kind of array an empty list will be. I guess it's just byte because it comes first in the enum-definition (or some similar, rather random).

[travipross]

I just encountered this issue and I am surprised this issue has been closed in favor of simply updating docs to state that it's broken.

Granted, perhaps the documentation update in https://github.com/ros2/rclpy/pull/957 is welcome and necessary at the moment, but it feels inappropriate for any empty list to be assumed to be of type BYTE_ARRAY simply because it is enumerated first here.

Is there truly no way to declare a STRING_ARRAY parameter with rclpy whose default value is an empty list? Not all empty arrays are BYTE_ARRAYS, and by ignoring any parameter type supplied by the user via the ParameterDescriptor argument, it creates a difficult-to-spot bug where it "fails" quite silently.

[fujitatomoya]

@travipross

Is there truly no way to declare a STRING_ARRAY parameter with rclpy whose default value is an empty list? Not all empty arrays are BYTE_ARRAYS, and by ignoring any parameter type supplied by the user via the ParameterDescriptor argument,

if you have concrete design or idea, i am happy to discuss and review.

it creates a difficult-to-spot bug where it "fails" quite silently.

i am not sure what you mean here, can you elaborate it? i was thinking that error can be obvious for type mismatch.

[travipross]

@fujitatomoya sorry for the vagueness of my comment. I'll try to elaborate:

i was thinking that error can be obvious for type mismatch.

The "error" that I found hard to catch was one where I had a parameter declared via rclpy.Node.declare_parameter() which was of type STRING_ARRAY and had some default value such as ["dog", "cat", "bird"]. With this node, I typically run with a launchfile and use a yaml parameters file with overrides (e.g. ["hello", "world"]).

At some point in the code, I call rclpy.Node.set_parameters() to set a new STRING_ARRAY type value. This executes normally and I can confirm that the parameter change was successful using many approaches, including ros2cli commands such as ros2 param get.

But when I change my default parameter value in the node, or the parameter value in my YAML file, to instead be an empty list [], this workflow no longer accepts the variable change. No exception is raised, but the parameter change fails "silently" due to the parameter type having been incorrectly initialized as a BYTE_ARRAY type (because the empty list was assumed to be a BYTE_ARRAY as per this check. I say "silently" because it would be expected that this sort of type mismatch should raise an exception if the parameter value passed in was invalid. Otherwise, this message regarding the failure is tucked away into an attribute of a response object. In web APIs, this would be like returning HTTP status code 200, but with the payload saying "failure".

if you have concrete design or idea, i am happy to discuss and review.

The core of the issue is that an empty array is a perfectly valid value for a parameter of type STRING_ARRAY (or any other array type) to assume. But it cannot be initialized with that value at declaration time because of two related reasons that I can see:

1. The rclpy.Node.declare_parameter() method accepts a ParameterDescriptor object as an argument but does not utilize it for assuming the type of the parameter being declared. Instead, this method calls rclpy.Node.declare_parameters() with a combination of arguments that causes the parameter type to be inferred from the default value that is provided, regardless of the type set by the user in the ParameterDescriptor.
2. Additionally, when inferring the type from the value itself, the logic used for checking the type will always assume any empty array is a BYTE_ARRAY. Without dynamic typing enabled, I hope it's clear how this can be problematic.

I'm not quite familiar enough with any cascading effects to suggest a solution immediately, or even necessarily what is the best behaviour in this case. But it certainly seems valid to keep this GitHub issue alive until some solution is put forward.

The issue is that all(isinstance(v, bytes) for v in parameter_value) will evaluate to True when parameter_value = []. But so will each of the other logical checks in that block of code. It's not testing whether or not the parameter_value is unambiguously a BYTE_ARRAY type, but yet it's enforcing that to be the case if it encounters an empty list. It could be any other *_ARRAY type as well, but it will always assume BYTE_ARRAY because it is the first potential candidate type that is being considered. It feels unintentionally ambiguous.

It may not be easy to determine the intended type of an empty python list, but for point 1 above, I feel that the declare_parameter() method should respect any parameter type specifications if they are provided via the ParameterDescriptor. This way the logical ambiguity in the rclpy.Parameter.Type.from_parameter_value() method can at least be avoided.

[fujitatomoya]

@travipross really appreciate your detailed explanation.

But when I change my default parameter value in the node, or the parameter value in my YAML file, to instead be an empty list [], this workflow no longer accepts the variable change.

that is reasonable. this can be surprise for user application.

regardless of the type set by the user in the ParameterDescriptor.

i believe this behavior is not bug cz that is intentionally implemented in that way. (not saying this is perfect solution, could be nicer for user experience.)

when inferring the type from the value itself, the logic used for checking the type will always assume any empty array is a BYTE_ARRAY.

probably this could be where we want to address this issue? as user point of view, [] is array but it could be any array? in that case, descriptor should be honored? and if there is no descriptor, we can fall back to default BYTE_ARRAY to keep the backward compatibility?

any thoughts? @clalancette @wjwwood @methylDragon @mjcarroll @sloretz

[travipross]

Could one valid solution be to simply add one more check in this block of the Node.declare_parameters() method like:

if not isinstance(value, ParameterValue):
    if descriptor.type is None:  # Only infer parameter type from value if not already specified in descriptor
        descriptor.type = Parameter.Type.from_parameter_value(value).value
else:
    if value.type == ParameterType.PARAMETER_NOT_SET:
        raise ValueError(
            'Cannot declare a statically typed parameter with default value '
            'of type PARAMETER_NOT_SET')
    descriptor.type = value.type

The way this method is called by Node.declare_parameter(), it is possible for descriptor.type to be already populated, and this block is otherwise just overwriting any potential type that could have been supplied by the user (as part of the 3 args passed to this method in Node.declare_parameter()).

[fujitatomoya]

@travipross thanks for iterating with me, lets keep this open for more feedback from other developers 👍 it would be better to get more information or ideas before implementation. (saying i may be missing something 😅 )

[travipross]

@fujitatomoya no problem! Is anyone able to change the status of this issue from Closed back to Open to indicate that it's still under discussion?

[diramazioni]

I've just started out using ROS, and, as a new user, I find really confusing that

• you explicitly set ParameterType.PARAMETER_STRING_ARRAY and is not honored
• [] defaults to BYTE_ARRAY this is weird, who use bytes as parameters?

[jdlangs]

I recently ran into this as well. IMO, the parameter type guessing here is erroneous because it doesn't account for the empty list case. As @travipross has been advocating, it should not just pick the first value checked because all([]) happens to return True and should either raise an exception or return some "type undetermined" flag so a different argument can be used.

As far as the issue in the OP, I don't think anyone posted that you can use a ParameterValue with STRING_ARRAY type and empty list as the declared default. This works fine for me:

self.declare_parameter('my_strings',
    rcl_interfaces.msg.ParameterValue(type=rcl_interfaces.msg.ParameterType.PARAMETER_STRING_ARRAY),
)

[fujitatomoya]

[] defaults to BYTE_ARRAY this is weird

i think all problems and complains come back to this point. [] empty list tuple and array will be deduced to BYTE_ARRAY now. this is because we do not have EMPTY_ARRAY type. but this actually means default value [] is set by user, so it would be okay to deduce to default (currently BYTE_ARRAY) array. and, deduction based on default value will be honored. i think this is designed behavior for a long time and it will be. see https://github.com/ros2/rclpy/pull/887#discussion_r796025172

besides, user can call declare_parameter without default value, in that case type and descriptor will be honored. (also https://github.com/ros2/rclpy/issues/912#issuecomment-1856314651)

    node.declare_parameter(
        #"str_list", [""], # this works as string array
        "str_list", [], # this deduced as byte array always
        #name = "str_list", # if default is not provided, rclpy applies the descriptor info
        ParameterDescriptor(
            type=ParameterType.PARAMETER_STRING_ARRAY,
            description="optionally some strings",
        )
    )

i am open for discussion on default deduction for [] since many people considers that would be surprising? but we need to be careful on this, changing default behavior easily breaks application behavior. IMO, i am not inclined to change the default for now.

any thoughts? @clalancette @wjwwood @methylDragon

[jackiescanlon]

I'm currently using get_parameter_or as a workaround for this issue:

self.declare_parameter(
    "myparam",
    descriptor=ParameterDescriptor(
        type=rcl_interfaces.msg.ParameterType.PARAMETER_INTEGER_ARRAY,
        description="myparam description",
    ),
)
param = self.get_parameter_or("myparam",
    rclpy.parameter.Parameter(
        "myparam",
        type_=rclpy.parameter.Parameter.Type.INTEGER_ARRAY,
        value=[],
    ),
).value

param will have value [] if myparam was not specified at node launch without getting the error about BYTE_ARRAY.

[tony-p]

@fujitatomoya as well as the suggestions to fix the default behaviour of empty list, rather than just ignoring the parameter descriptor type when there is a default value, doesn't it make sense to validate that the inferred type from the default value matches the type defined in the descriptor (when provided) rather than just overwriting it?

If the default value is an empty collection (list), it should also be able to adjust the type with respect to the parameter descriptor before setting the parameter.

As for concerns about breaking current applications, this should at least be able to be robustly fixed in rolling to ensure it doesn't persist longer