8000 Setting _filters to None when filter Iterator is empty by pierreluctg · Pull Request #325 · hardbyte/python-can · GitHub
[go: up one dir, main page]

Skip to content

Setting _filters to None when filter Iterator is empty#325

Merged
felixdivo merged 4 commits intohardbyte:developfrom
pierreluctg:patch-1
Jun 13, 2018
Merged

Setting _filters to None when filter Iterator is empty#325
felixdivo merged 4 commits intohardbyte:developfrom
pierreluctg:patch-1

Conversation

@pierreluctg
Copy link
Collaborator

For example when using can.logger the default can_filters value is a empty list, this leads to filtering all message. This change fix the issue by treating any empty Iterator same as filters=None

For example when using `can.logger` the default filter value is a empty list, this leads to filtering all message. This change fix the issue by treating any empty Iterator as filter==None
@pierreluctg
Copy link
Collaborator Author
pierreluctg commented Jun 12, 2018

@hardbyte What is your opinion on that one?

Currently this PR is not passing the match_nothing test in https://github.com/hardbyte/python-can/blob/develop/test/test_message_filtering.py#L49 intruduced by @felixdivo in #277

I am not sure is this test make sense or not. When looking at the interfaces that supports filtering directly, they treat a empty Iterator as no filtering (opposite of the test)

@felixdivo felixdivo added the api label Jun 12, 2018
@felixdivo felixdivo requested a review from hardbyte June 12, 2018 16:59
@felixdivo
Copy link
Collaborator

When I implemented that API, I did not find any precise or central documentation on how to handle can_filters. I simply have treated empty iterables like I thought it to be the most intuitive. That's the story behind the change I made.

@felixdivo
Copy link
Collaborator

When this is actually changed, please update the documentation accordingly.

@pierreluctg
Copy link
Collaborator Author

Tests and doc have been updated

can/bus.py Outdated
All messages that match at least one filter are returned.
If `filters` is `None`, all messages are matched.
If it is a zero size interable, no messages are matched.
If `filters` is `None` or a a zero size interable, all
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo here. I'd say "a zero length sequence"

@felixdivo felixdivo merged commit e6d9e76 into hardbyte:develop Jun 13, 2018
@pierreluctg pierreluctg deleted the patch-1 branch August 14, 2018 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0