asyml / asyml/forte

Require a better way to initialize reader by given config

Open
#334 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.