add warning when queue_size is omitted for rospy publisher

[dirk-thomas]

The warning can be suppressed by setting:

export PYTHONWARNINGS="ignore::SyntaxWarning"

I choose SyntaxWarning since I wanted to make it distinguishable from the existing warnings in rospy (which are by default UserWarnings).

This will lead to a lot of warning for every existing Python code out there. I am just saying this explicitly. We should consider that and also might want to ask the community about that?

@esteve @tfoote @wjwwood Please review and provide feedback.

[jonbinney]

Should the rosout queue_size param be exposed somewhere as an argument with a default value? Users on bad wifi who really don't want to drop debugging output might want something higher than 100.

[esteve]

LGTM and I agree with asking the community about it. Should we also file tickets on ROS packages that use rospy and omit the queue_size keyword? I know it's not really our responsibility fixing other people's code, but if I were a user, I'd like a sane user experience without warnings every time I use someone else's code.

[wjwwood]

I think that this is fine. We should get some feedback from the community.

[wjwwood]

Is commit 53a2213 supposed to be a part of this pull request?

[tkruse]

IMO the warning message should hint at why a queue_size param of None is not desirable, rather than just telling users not to use it. If we say a value of zero is dangerous, but we agree that a value if None is worse, then it would be weird to have a stronger warning about zero than about None. A code comment (not the warning) could explain that the behavior of None was historically chosen but proved to cause problems.

Also as said on ros-user I think the invocation with param value none should be dreprecated, to be replaced by ValueError in some distant future, not necessarily the following distro.

Additionally, I would suggest putting in a code comment pointing to this github issue and/or explaining the and future plans around this (in particular when deprecating).

Imagine you read the code as a new ROS user without having read this github issue before, and ask yourself whether the code (or warning) provide sufficient information to explain the API.

[dirk-thomas]

I have updated the warning message to state that the new keyword should be passed as well as a reference to more information in the wiki.

[dirk-thomas]

Please provide final feedback on the updated code changes - especially the wording of the doc block and the warning message - before being merged.

[wjwwood]

+1

[jonbinney]

+1