Repository: pytorch/ignite

Issue:
Add args/kwargs to `BaseLogger.attach` method
## 🚀 Feature

Following this question: https://github.com/pytorch/ignite/discussions/2031 we can easily propagate args/kwargs for custom handlers:
```diff
def attach(
-     self, engine: Engine, log_handler: Callable, event_name: Union[str, Events, CallableEventWithFilter, EventsList]
+    self, engine: Engine, log_handler: Callable, event_name: Union[str, Events, CallableEventWithFilter, EventsList], *args, **kwargs
):
...

- engine.add_event_handler(event_name, log_handler, self, event_name)
+ engine.add_event_handler(event_name, log_handler, self, event_name, *args, **kwargs)
```

Discussion:
sdesrozis: @afzal442 Thank you ! Could you add a test please ?
afzal442: > Could you add a test please ?

Thanks. Need help with that plz.

afzal442: https://github.com/pytorch/ignite/blob/master/tests/ignite/contrib/handlers/test_base_logger.py
sdesrozis: > https://github.com/pytorch/ignite/blob/master/tests/ignite/contrib/handlers/test_base_logger.py

L120, L179 the tests using MagicMock as log_handler could be improved by adding extra args / kwargs. Thus, testing means verify that the log_handler has been called with the correct arguments, see L183.

HTH
afzal442: Thanks @sdesrozis , that makes sense. 


sdesrozis: We need first a few specific tests to validate this new feature. The lines you mentioned could be updated, yet this is not mandatory.
vfdev-5: @afzal442 yes, in `test_attach()` we can add another sub-test case : 
```python
_test(Events.ITERATION_STARTED, len(data) * n_epochs, kwargs={"a": 234})
```
and update `_test` method as 
```python
def _test(event, n_calls, kwargs={}):
    ...
    logger.attach(trainer, log_handler=mock_log_handler, event_name=event, **kwargs)
    ...
    if len(kwargs) > 0:
        calls = [call(trainer, logger, e, **kwargs) for e in events]
    else:
        calls = [call(trainer, logger, e) for e in events]
    ...
```
afzal442: Thanks @vfdev-5, could you tell me why we declare this as `'a' : 234` not `Any`?

`_test(Events.ITERATION_STARTED, len(data) * n_epochs, kwargs={"a": 234})`

vfdev-5: > Thanks @vfdev-5, could you tell me why we declare this as `'a' : 234` not `Any`?
> 
> `_test(Events.ITERATION_STARTED, len(data) * n_epochs, kwargs={"a": 234})`

@afzal442 it is just an example I provided in the previous message. I'm not sure to understand what you mean by "not `Any`", values of kwargs can not be `Any`.

KickItLikeShika: Hey @afzal442, I'd like to help you to finalize this PR please, we can talk on Discord if you'd like.
KickItLikeShika: The updates are fine now, and there's nothing wrong about them, let's wait until @vfdev-5 or @sdesrozis restart the GA workflow

Patch:
diff --git a/ignite/contrib/handlers/base_logger.py b/ignite/contrib/handlers/base_logger.py
index 60179db9..edbc2538 100644
--- a/ignite/contrib/handlers/base_logger.py
+++ b/ignite/contrib/handlers/base_logger.py
@@ -151,7 +151,12 @@ class BaseLogger(metaclass=ABCMeta):
     """
 
     def attach(
-        self, engine: Engine, log_handler: Callable, event_name: Union[str, Events, CallableEventWithFilter, EventsList]
+        self,
+        engine: Engine,
+        log_handler: Callable,
+        event_name: Union[str, Events, CallableEventWithFilter, EventsList],
+        *args: Any,
+        **kwargs: Any,
     ) -> RemovableEventHandle:
         """Attach the logger to the engine and execute `log_handler` function at `event_name` events.
 
@@ -161,6 +166,8 @@ class BaseLogger(metaclass=ABCMeta):
             event_name: event to attach the logging handler to. Valid events are from
                 :class:`~ignite.engine.events.Events` or :class:`~ignite.engine.events.EventsList` or any `event_name`
                 added by :meth:`~ignite.engine.engine.Engine.register_events`.
+            args: args forwarded to the `log_handler` method
+            kwargs: kwargs forwarded to the  `log_handler` method
 
         Returns:
             :class:`~ignite.engine.events.RemovableEventHandle`, which can be used to remove the handler.
@@ -178,7 +185,7 @@ class BaseLogger(metaclass=ABCMeta):
             if event_name not in State.event_to_attr:
                 raise RuntimeError(f"Unknown event name '{event_name}'")
 
-            return engine.add_event_handler(event_name, log_handler, self, event_name)
+            return engine.add_event_handler(event_name, log_handler, self, event_name, *args, **kwargs)
 
     def attach_output_handler(self, engine: Engine, event_name: Any, *args: Any, **kwargs: Any) -> RemovableEventHandle:
         """Shortcut method to attach `OutputHandler` to the logger.


Test patch:
diff --git a/tests/ignite/contrib/handlers/test_base_logger.py b/tests/ignite/contrib/handlers/test_base_logger.py
index 02aeabcb..ed5a5a37 100644
--- a/tests/ignite/contrib/handlers/test_base_logger.py
+++ b/tests/ignite/contrib/handlers/test_base_logger.py
@@ -103,7 +103,7 @@ def test_attach():
     n_epochs = 5
     data = list(range(50))
 
-    def _test(event, n_calls):
+    def _test(event, n_calls, kwargs={}):
 
         losses = torch.rand(n_epochs * len(data))
         losses_iter = iter(losses)
@@ -117,7 +117,7 @@ def test_attach():
 
         mock_log_handler = MagicMock()
 
-        logger.attach(trainer, log_handler=mock_log_handler, event_name=event)
+        logger.attach(trainer, log_handler=mock_log_handler, event_name=event, **kwargs)
 
         trainer.run(data, max_epochs=n_epochs)
 
@@ -125,11 +125,16 @@ def test_attach():
             events = [e for e in event]
         else:
             events = [event]
-        calls = [call(trainer, logger, e) for e in events]
+
+        if len(kwargs) > 0:
+            calls = [call(trainer, logger, e, **kwargs) for e in events]
+        else:
+            calls = [call(trainer, logger, e) for e in events]
+
         mock_log_handler.assert_has_calls(calls)
         assert mock_log_handler.call_count == n_calls
 
-    _test(Events.ITERATION_STARTED, len(data) * n_epochs)
+    _test(Events.ITERATION_STARTED, len(data) * n_epochs, kwargs={"a": 0})
     _test(Events.ITERATION_COMPLETED, len(data) * n_epochs)
     _test(Events.EPOCH_STARTED, n_epochs)
     _test(Events.EPOCH_COMPLETED, n_epochs)