In the test we still however want to test __getattr__, so let's keep mock_logger.add_scalar instead of mock_logger.writer.add_scalar and others where we call mock_logger.writer.some_func -> mock_logger.some_func

Ok let me make the change

Hey I did that and it failed

OK, mock does not have __getattr__ replicated from the spec. Let's find another way to test that user can access methods like writer.add_scalar directly using logger.add_scalar. Do you have any ideas or suggestions ?

I guess we can have a multiple inheritance in the tensorboard logger such that it supports all methods without the writer instance variable. I am testing a few things in this area.

here is the dict of tensorboard logger when I added SummaryWriter as second inheritance in TensorboardLogger class .
{'_mock_return_value': sentinel.DEFAULT, '_mock_parent': None, '_mock_name': None, '_mock_new_name': '', '_mock_new_parent': None, '_mock_sealed': False, '_spec_class': <class 'ignite.contrib.handlers.tensorboard_logger.TensorboardLogger'>, '_spec_set': None, '_spec_signature': <Signature (*args: Any, **kwargs: Any)>, '_mock_methods': ['_SummaryWriter__append_to_scalar_dict', 'abstractmethods', 'class', 'delattr', 'dict', 'dir', 'doc', 'enter', 'eq', 'exit', 'format', 'ge', 'getattr', 'getattribute', 'gt', 'hash', 'init', 'init_subclass', 'le', 'lt', 'module', 'ne', 'new', 'reduce', 'reduce_ex', 'repr', 'setattr', 'sizeof', 'str', 'subclasshook', 'weakref', '_abc_impl', '_check_caffe2_blob', '_create_opt_params_handler', '_create_output_handler', '_encode', '_get_comet_logger', '_get_file_writer', 'add_audio', 'add_custom_scalars', 'add_custom_scalars_marginchart', 'add_custom_scalars_multilinechart', 'add_embedding', 'add_figure', 'add_graph', 'add_graph_deprecated', 'add_histogram', 'add_histogram_raw', 'add_hparams', 'add_image', 'add_image_with_boxes', 'add_images', 'add_mesh', 'add_onnx_graph', 'add_openvino_graph', 'add_pr_curve', 'add_pr_curve_raw', 'add_scalar', 'add_scalars', 'add_text', 'add_video', 'attach', 'attach_opt_params_handler', 'attach_output_handler', 'close', 'export_scalars_to_json', 'flush'], '_spec_asyncs': [], '_mock_children': {'writer': }, '_mock_wraps': None, '_mock_delegate': None, '_mock_called': False, '_mock_call_args': None, '_mock_call_count': 0, '_mock_call_args_list': [], '_mock_mock_calls': [], 'method_calls': [], '_mock_unsafe': False, '_mock_side_effect': None, 'writer': }

I guess through this we can remove the need of writer.add_scalar

Yes we could inherit also from SummaryWriter but this would imply tensorboard package as static ignite dependency vs right now it is a runtime dependency and TensorboardLogger class is exposed in ignite even if tensorboard package is not installed...

I think we can proceed with __getattr__ solution and just add a specific test such that we can ensure that methods logger.add_scalar is propagated to logger.writer.add_scalar.

I guess you are right, get_attr is returning the self.writer methods but Magic block doesn't seem to recognise it so when I printed the dict of a tensorboardlogger I got the dict of the SummaryWriter

Let's revert this line as well

We are using pytest so let's stick to their formalism without creating TestCase classes and just directly defining test functions.

We need to check that logger.writer.add_scalar was called with ("loss", 0.5)