Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation vs. code: Clarify and/or fix logic of "filter" function #637

Open
amotl opened this issue Apr 14, 2023 · 3 comments
Open

Documentation vs. code: Clarify and/or fix logic of "filter" function #637

amotl opened this issue Apr 14, 2023 · 3 comments
Labels
bug help wanted quality All things QA, software tests, and maintenance

Comments

@amotl
Copy link
Member

amotl commented Apr 14, 2023

Dear JP and Ben,

this is an important issue. Maybe you can find a few minutes to look into it.

With kind regards,
Andreas.

Introduction

I am bringing this up, because @sevmonster shared their experiences when using mqttwarn's filter feature, and because, after comparing it with the documentation, I discovered an anomaly between mqttwarn's documentation and implementation.

In its current incarnation, should it not be stopping all messages since it always returns False? Why are messages getting through? Am I misunderstanding the filter function usage?

I did individually test the logic in the conditional with dummy data so the rest of it should be sound and do what I want, with the understanding that True is a passing filter and False fails.

-- #632 (comment)

Documentation

While working on GH-635, I am now at the spot where the documentation states:

A function called from the filter property in a topic section needs to return False to stop the outbound notification.

-- Custom functions » Filter functions

Observations

However, while working on GH-632 and GH-635, we discovered that the opposite might be the case. Both code examples actually demonstrate that you need to return True to stop the outbound notification.

@amotl
Copy link
Member Author

amotl commented Apr 14, 2023

mqttwarn's documentation would match Python's filter function 1:

filter(function, iterable): Construct an iterator from those elements of iterable for which function is true.

However, mqttwarn's implementation is apparently doing it the other way round, contrary to its documentation. Is it a bug? How would we approach fixing this?

Footnotes

  1. https://docs.python.org/3/library/functions.html#filter

@amotl amotl changed the title Documentation vs. code: Clarify logic of "filter" function Documentation vs. code: Clarify and/or fix logic of "filter" function Apr 14, 2023
@amotl amotl added bug help wanted quality All things QA, software tests, and maintenance labels Apr 14, 2023
@codebude
Copy link

I also stumbled about this. I would suggest to keep the implementation as it is (to not break implementation done by all the users), but update the documentation to be more precise on how to create the filter.

E.g.:

A function called from the filter property in a topic section needs to return True to stop the outbound notification

@amotl
Copy link
Member Author

amotl commented May 22, 2023

Documentation fixed with 288392f. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted quality All things QA, software tests, and maintenance
Projects
None yet
Development

No branches or pull requests

2 participants