Require a better way to initialize reader by given config
- Dominant language
- Python
- Stars
- 253
- Forks
- 59
- PR merge metrics
- No merged PRs in 30d
Description
**Is your feature request related to a problem? Please describe.**
Currently `BaseReader::__init__` method have several arguments like `from_cache`, `cache_directory`, etc. and those parameters are optional. This will introduce some potential risks:
* Some child readers inherited from base reader will overwrite `__init__` without provide any base reader level argument (and no bug detected by modern IDE as those args are optional)
* reader class also provides a `initialize` method to do initialization. Those two entry points will introduce confusion.
* This is not flexible as if we want to suport more option, we need to add another arg in `BaseReader::__init__`.
It would be better if we entirely leave `BaseReader::__init__` as empty and move all args into `default_configs`. And then we only do initialization in `initialize` method.
However, this also means any developer who wants to write a new reader need to keep in mind that the `super().initialize()` should be called in `initialize` and `super().default_configs` should be called in `default_configs`. This is more error-prone.
**Describe the solution you'd like**
we need a cleaner and less error-prone way to do reader initialization.
**Describe alternatives you've considered**
Currently all child readers should not overwrite `BaseReader::__init__` or overwrite it but need to accept all arguments in `BaseReader::__init__`.
Contributor guide
Research direction
No file or test is named. Start by locating BaseReader and its __init__, initialize, and default_configs methods, then inspect child readers that override initialization. Done means the reader initialization API is made cleaner and less error-prone, with the required behavior covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100