Added args and kwargs to rospy.log* #1289
Conversation
clients/rospy/src/rospy/core.py
Outdated
| if name: | ||
| rospy_logger = rospy_logger.getChild(name) | ||
| del kwargs['logger_name'] |
There was a problem hiding this comment.
Instead of kwargs.get(...) and del kwargs[...] could this continue to call kwargs.pop(...) as it was before?
mgrrx
force-pushed
the
log-kwargs
branch
2 times, most recently
from
2aa6f73 to
ba0df3f
Compare
|
Rebased against melodic-devel and changed what was commented. |
| level = kwargs.pop('logger_level', None) | ||
| once = kwargs.pop('logger_once', False) | ||
| throttle_identical = kwargs.pop('logger_throttle_identical', False) | ||
| def _base_logger(msg, args, kwargs, throttle=None, |
There was a problem hiding this comment.
I don't see why args are kwargs can't continue to be passed as *args and **kwargs. What is the benefit wrapping them into a single parameter?
There was a problem hiding this comment.
Since the logger functions (logdebug, loginfo, ...) accept *args and **kwargs and _base_logger has additional optional arguments I wanted to clearly separate them. It's a somewhat artificial case but let's assume one would write the following code:
rospy.loginfo("foo", once=True, throttle=rospy.Duration(1.0), level='fatal')
This would make no sense at all but would break the logic in _base_logger.
For me it only makes sense to set change the logger_name when calling rospy.log*.
|
Any chance that this pull request will be merged? I stumbled over the limitations of |
|
@dirk-thomas which change is required here? I thought I addressed all issues |
That is why I removed the label. |
|
@ros-pull-request-builder retest this please |
|
Thanks for the patch. |
Implements #1222
I changed the signature of
_base_loggerso that e.g. a key namedlogger_levelcan be used for formattingmsg.