In this PR, I address the issue described in issue #156. The logic to fix this issue includes:
Removing header name and dtype checking from the ReadTabular._check_parameter( .. ) function.
Rename ReadTabular._check_parameter( .. ) to ReadTabular._convert_wide_2_narrow( .. ) as the logic of this function has been reduced to only convert wide format data to narrow data with no checks against the config file
Add header and dtype checks to the ReadStrategy._check_index( .. ) method. This includes breaking out the checking of dtypes and header names into separate functions, which ReadStrategy._check_index( .. ) calls.
Updated and added tests to reflect these changes
Updated the excel fixture, as it turns out there were a couple errors in there that otoole now catches haha
As mentioned in the comment in issue #156, I decided to move up the checking of header names and dtypes to the base ReadStrategy class. While this means there is an efficiency hit (since we read in all data then check the headers/dtypes, rather checking one by one as we read in the data), all current and future read strategies can now use the ReadStrategy._check_index( .. ) method! So I thought this trade off seemed okay.
If you have any comments @willu47, please just let me know! Else, I will go ahead a merge in :)
In this PR, I address the issue described in issue #156. The logic to fix this issue includes:
ReadTabular._check_parameter( .. )function.ReadTabular._check_parameter( .. )toReadTabular._convert_wide_2_narrow( .. )as the logic of this function has been reduced to only convert wide format data to narrow data with no checks against the config fileReadStrategy._check_index( .. )method. This includes breaking out the checking of dtypes and header names into separate functions, whichReadStrategy._check_index( .. )calls.As mentioned in the comment in issue #156, I decided to move up the checking of header names and dtypes to the base
ReadStrategyclass. While this means there is an efficiency hit (since we read in all data then check the headers/dtypes, rather checking one by one as we read in the data), all current and future read strategies can now use theReadStrategy._check_index( .. )method! So I thought this trade off seemed okay.If you have any comments @willu47, please just let me know! Else, I will go ahead a merge in :)