Cleanup DataConfig implementation

[shaahji]

Cleanup DataConfig implementation

• Removed DataConfig::params_config
• Removed DataConfig::components/component_args

All components specific parameters are now grouped in four separate objects:

• load_dataset_config
• pre_process_data_config
• post_process_data_config
• dataloader_config

Checklist before requesting a review

• Add unit tests for this change.
• Make sure all tests can pass.
• Update documents if necessary.
• Lint and apply fixes to your code by running lintrunner -a
• Is this a user-facing change? If yes, give a description of this change to be included in the release notes.
• Is this PR including examples changes? If yes, please remember to update example documentation in a follow-up PR.

(Optional) Issue link

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[guotuofeng]

do we really need the params key name? it would be better if like

            "load_dataset_config": {
                    "data_name": "glue",
                    "subset": "mrpc",
                    "split": "validation"
            },

[guotuofeng]

the type info is needless since the load_dataset_config already contains the type info

[shaahji]

do we really need the params key name? it would be better if like

            "load_dataset_config": {
                    "data_name": "glue",
                    "subset": "mrpc",
                    "split": "validation"
            },

I agree that params seems redundant. I wasn't sure, what would the config class look like since type is required parameter and other are all optional. There is no strict list of what the params can be (think of custom data set/ data container implementation which can take any user arguments).

[shaahji]

the type info is needless since the load_dataset_config already contains the type info

Hmm, I am unsure what you mean config already contains the type info. Please elaborate.

[jambayk]

I also agree params nesting is not needed. But like Hitesh said, type field is still needed if the user wants to override the default component type.

[shaahji]

If the changes in this PR look good, I would like to merge this change before making the decorative params change. It's modifying too many files, and I don't want to sit on them for too long. Merging will be very hard.

[jambayk]

sounds good to me! as you said, it's a cosmetic change that only looks different on the user side. In the backend, we need a way to hold the params anyways.

[guotuofeng]

Is the type required? I don't see it in the example.
what I mean is not change the DataComponentConfig and just convert the plain json dict to DataComponentConfig in validator.

[jambayk]

example of type override

[shaahji]

Even without that params is still required to collect only the parameters that are passed down to the creator function. I noticed that DataSet/DataContainer constructors fail if they are passed unknown arguments (including in kwargs). So, it might not actually be possible to remove the params entirely. If we did, we will still be doing custom include/exclude list to collect only the arguments that are passed down to the constructor (but then what are the other parameters for? discard?).