Skip to content

Better python filter-class interfaces #186

@EsipovPA

Description

@EsipovPA

Description

There are a few things, that seem like they may be done in a more python-way IMHO.

Subscriber

Subscriber filter __init__ signature looks like this:

def __init__(self, *args, **kwargs):

The *args are decomposed into positional arguments.

    def __init__(self, *args, **kwargs):
        SimpleFilter.__init__(self)
        self.node = args[0]
        self.topic = args[2]

As a result, to initialize Subscriber filter, one has to pass node name, topic and message type as positional arguments. And the expected order of the positional arguments is not provided by the method signature, or by the class help. But the QoS profile can only be passed as a keyword argument. So the correct way to initialize the Subscriber filter looks like this:

            self.subscriber_filter = Subscriber(
                self,
                String,
                "/example/topic",
                qos_profile=qos_profile,
            )

It is the only filter in this library, that utilizes this approach. Others use named arguments. Maybe it is better to refactor the signature?

Extremely short argument names and parameter names in come of the filter classes in general.

For example

    def __init__(self, fs, queue_size):

fs instead of filters, or filters_list. And some other examples. The TimeSequencer class seems to be in the best condition in that way. Maybe It will be better to refactor other classes with the same design philosophy in mind, as the TimeSequencer class?

If nobody is working on this matter, I would like to try my best and provide PR with corresponding changes.

Motivation

Better code readability. Simpler long term code support.

Design / Implementation Considerations

Backwards compatibility.

Additional Information

No response

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requesthelp wantedExtra attention is needed

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions