Hi @sandraalnajjar —thank you very much for your contributions to the iTEM Open Data code! We appreciate that members of the community, such as yourself, make efforts to expand the scope and quality of the data included in this shared resource.
In this review, I will do a few things:
Summarize what I think is being added by the commits on this branch. (This is information that should go in the description of the pull request. If I am guessing incorrectly, please correct me by providing the correct explanation.)
Identify TODOs to merge this branch/PR.
Identify TODOs for further clean-up (after the branch is merged).
To be clear, I understand from you and @soniayeh that the course in which you did this work is coming to an end, so you may not have dedicated time to do (2) or (3). So those TODOs are mentioned without saying who will do them or when—it could be you, me, or someone else; sooner or later.
What is added
Three data-cleaning modules, each cleaning one data flow from the Asian Development Bank (ADB) Asian Transport Outlook (ATO); I guess the 2023 edition:
item.historical.T021 → TAS-PAG-005(3)
item.historical.T022 → TAS-VEP-017
item.historical.T023 → TAS-FRA-007(2)
TODOs to merge the branch
[ ] Deduplicate the code in the 3 added modules. I see that the modules T022 and T023 both contain class AtoWorkbook with many common lines; this makes it difficult to identify which steps are common versus specific to each input data flow. T021 appears to contain a class ItemTransformer with a different structure. See also the second TODO in the next section.
[ ] Provide a single entry-point in each module. Existing data modules like T000 do not execute any code when they are imported; they instead provide a function like process() that triggers processing of the data.
[ ] Add documentation, for instance in the docstring at the top of each module. Explain, at least, which specific data are used: at what URL can the original input data be found?
[ ] Ensure the tests pass. This can be started by rebasing the branch on transportenergy:main.
TODOs for follow-up
[ ] Consider overlap with the transport_data.adb code maintained by the Transport Data Commons, here; make use of the TDC code where possible, and keep the features specific to iTEM Open Data.
[ ] Consider overlap with e.g. the class AtoWorkbook added in #78 and #81 and the class ItemTransformer added in #80; merge common actions to common code.
[ ] Add the new modules to the test suite. item.tests.test_historical contains tests of the existing (T000–T010, T012) data modules, and these tests should be expanded to test the modules added in this PR.
Hi @sandraalnajjar —thank you very much for your contributions to the iTEM Open Data code! We appreciate that members of the community, such as yourself, make efforts to expand the scope and quality of the data included in this shared resource.
In this review, I will do a few things:
To be clear, I understand from you and @soniayeh that the course in which you did this work is coming to an end, so you may not have dedicated time to do (2) or (3). So those TODOs are mentioned without saying who will do them or when—it could be you, me, or someone else; sooner or later.
What is added
item.historical.T021
→ TAS-PAG-005(3)item.historical.T022
→ TAS-VEP-017item.historical.T023
→ TAS-FRA-007(2)TODOs to merge the branch
class AtoWorkbook
with many common lines; this makes it difficult to identify which steps are common versus specific to each input data flow. T021 appears to contain aclass ItemTransformer
with a different structure. See also the second TODO in the next section.T000
do not execute any code when they are imported; they instead provide a function likeprocess()
that triggers processing of the data.transportenergy:main
.TODOs for follow-up
transport_data.adb
code maintained by the Transport Data Commons, here; make use of the TDC code where possible, and keep the features specific to iTEM Open Data.class AtoWorkbook
added in #78 and #81 and theclass ItemTransformer
added in #80; merge common actions to common code.item.tests.test_historical
contains tests of the existing (T000–T010, T012) data modules, and these tests should be expanded to test the modules added in this PR.