fix Bug: KeyError: 'text' Corresponding to issue #296

[shiweijiezero]

Solution for issue #296
File data_juice/config/config.py lines 418-429 did not consider the situation when arg: text_key was initialized to 'text', resulting in arg: text_key not being updated properly and always being initialized to the value of 'text'
Thus, I just supplement the set of "text_key".

[shiweijiezero]

Original Code:

    for op in cfg.process:
        for op_name in op:
            args = op[op_name]
            if args is None:
                args = {
                    'text_key': text_key,
                    'image_key': cfg.image_key,
                    'audio_key': cfg.audio_key,
                    'video_key': cfg.video_key,
                }
            elif args['text_key'] is None:
                args['text_key'] = text_key
                args['image_key'] = cfg.image_key
                args['audio_key'] = cfg.audio_key
                args['video_key'] = cfg.video_key

            op[op_name] = args

Fixed:

    for op in cfg.process:
        for op_name in op:
            args = op[op_name]
            if args is None:
                args = {
                    'text_key': text_key,
                    'image_key': cfg.image_key,
                    'audio_key': cfg.audio_key,
                    'video_key': cfg.video_key,
                }
            elif args['text_key'] is None:
                args['text_key'] = text_key
                args['image_key'] = cfg.image_key
                args['audio_key'] = cfg.audio_key
                args['video_key'] = cfg.video_key
            if text_key:
                args['text_key'] = text_key
                args['image_key'] = cfg.image_key
                args['audio_key'] = cfg.audio_key
                args['video_key'] = cfg.video_key

            op[op_name] = args

[HYLcool]

Hi @shiweijiezero , thanks for your contribution!

You're correct👍🏻. According to your report and modification, we found out that a previous PR #165 made the code snippet useless because args for each OP are initialized by the default args and command line args before.

data-juicer/data_juicer/config/config.py

Lines 418 to 434 in 094440b

	if args is None:
	args = {
	'text_key': text_key,
	'image_key': cfg.image_key,
	'audio_key': cfg.audio_key,
	'video_key': cfg.video_key,
	}
	elif args['text_key'] is None:
	args['text_key'] = text_key
	args['image_key'] = cfg.image_key
	args['audio_key'] = cfg.audio_key
	args['video_key'] = cfg.video_key
	if text_key:
	args['text_key'] = text_key
	args['image_key'] = cfg.image_key
	args['audio_key'] = cfg.audio_key
	args['video_key'] = cfg.video_key

In the data-juicer config modules, args of OPs are expected to be overwritten in this priority order: default args -> config args -> command line args. Following this rule and based on your modification, we suggest you continue to fix this bug completely by:

1. swapping these two lines 271 and 272 of code. Function init_setup_from_cfg updates OP args from config files and function update_op_process updates them from command line args. So we need to apply init_setup_from_cfg then update_op_process.
   

   data-juicer/data_juicer/config/config.py

   Lines 269 to 273 in 094440b

   	try:
   	cfg = parser.parse_args(args=args)
   	cfg = update_op_process(cfg, parser)
   	cfg = init_setup_from_cfg(cfg)

2. decoupling the updates for text/audio/image/video keys. These keys are updated only by the changes of text_key before, which would cause the same issue you met on audio/image/video keys. So we suggest you modify this part (lines 425 to 429) of the code like the below:

            ......
            else:
                if 'text_key' not in args or args['text_key'] is None:
                    args['text_key'] = text_key
                if 'image_key' not in args or args['image_key'] is None:
                    args['image_key'] = cfg.image_key
                if 'audio_key' not in args or args['audio_key'] is None:
                    args['audio_key'] = cfg.audio_key
                if 'video_key' not in args or args['video_key'] is None:
                    args['video_key'] = cfg.video_key
            op[op_name] = args
            ......

Thanks for your contribution again! Feel free to discuss the fix suggestions with us if you have any further considerations!

[shiweijiezero]

Yeah,
Following your suggestion, everything seems to be OK.
And I had committed it again.

[HYLcool]

One last thing, you might need to apply pre-commit checking according to coding style doc.

[HYLcool]

Nice. Thanks for your contribution!