In this PR I implement logic to deprecate the reading and writing of datapackages (issue #135). If a user tries to read/write to/from a datapackage, otoole tries to handle this by diverting to the ReadCSV and WriteCSV classes. A new utility function is added to handle the logic, and a new OtooleDeprecationError is added. Tests are added for the new utility function.
NOTE: This PR assumes the reading of CSVs bug addressed in PR #140 is implemented.
Logic
Reading
If a datapackage is selected to be read from, otoole will print a deprecation warning and try to find the data folder of CSVs. If the folder data/ exisits at the same directory level as datapackage.json, otoole will update the from_file argument and instantiate the ReadCSV class. If the folder data/ does not exist, an OtooleDeprecationError is raised.
Possible Issues
If an existing workflow has a custom datapackage.json that points to a folder of CSVs not named dataAND they also have a folder called data, this solution will try to read incorrect data. I think this is a edge case that probably wont be an issue, but definitively worth a second opinion!
Example
If the data/ folder exists at the same level as datapackage.json
(otoole) ~/repositories/otoole/trevor$ otoole convert datapackage datafile datapackage.json data.txt config.yaml
Reading from datapackage is deprecated, trying to read from CSVs
(otoole) ~/repositories/otoole/trevor$
If the data/ folder does not exist
(otoole) ~/repositories/otoole/trevor$ otoole convert datapackage datafile datapackage.json data.txt config.yaml
Reading from datapackage is deprecated, trying to read from CSVs
OtooleDeprecationError: datapackage.json -> datapackage format no longer supported and no csv data found
Writing
If a datapackage is written to, otoole will print a deprecation warning and write to a folder of CSVs called data/ instead.
Possible Issues
If the user has a folder called data/ already, it will be overwritten with the new data without warning.
Example
(otoole) ~/repositories/otoole/trevor$ otoole convert datafile datapackage data.txt datapackage.json config.yaml
Writing to datapackage is deprecated, writing to CSVs
(otoole) ~/repositories/otoole/trevor$
Questions
When writing, do you think we should add a chack to see if a folder called data/ already exists? I didn't do this check because in most cases (unless the datapackage is modified) they should expect a folder called data to appear?
I haven't deleted any of the datapackage code, as the issue mentioned we should keep it for when/if the datapackage functionality is added back in the future. I wasn't sure if just leaving non-accessible code in with the production code is okay or not? Moreover, should tests relating to the reading/writing of datapackages be moved/deleted/commented out?
NOTE: As the issue mentioned, the frictionless datapackage should no longer be required. I haven't removed the package from requirements though, as the datapackage code still references it; therefore, linitng checks would flag this. After discussing question 2, hopefully, the answer to this comment will be clear :)
Overview
In this PR I implement logic to deprecate the reading and writing of datapackages (issue #135). If a user tries to read/write to/from a datapackage,
otooletries to handle this by diverting to theReadCSVandWriteCSVclasses. A new utility function is added to handle the logic, and a newOtooleDeprecationErroris added. Tests are added for the new utility function.NOTE: This PR assumes the reading of CSVs bug addressed in PR #140 is implemented.
Logic
Reading
If a datapackage is selected to be read from,
otoolewill print a deprecation warning and try to find the data folder of CSVs. If the folderdata/exisits at the same directory level asdatapackage.json,otoolewill update thefrom_fileargument and instantiate theReadCSVclass. If the folderdata/does not exist, anOtooleDeprecationErroris raised.Possible Issues
If an existing workflow has a custom
datapackage.jsonthat points to a folder of CSVs not nameddataAND they also have a folder calleddata, this solution will try to read incorrect data. I think this is a edge case that probably wont be an issue, but definitively worth a second opinion!Example
If the
data/folder exists at the same level asdatapackage.jsonIf the
data/folder does not existWriting
If a datapackage is written to,
otoolewill print a deprecation warning and write to a folder of CSVs calleddata/instead.Possible Issues
If the user has a folder called
data/already, it will be overwritten with the new data without warning.Example
Questions
When writing, do you think we should add a chack to see if a folder called
data/already exists? I didn't do this check because in most cases (unless the datapackage is modified) they should expect a folder calleddatato appear?I haven't deleted any of the datapackage code, as the issue mentioned we should keep it for when/if the datapackage functionality is added back in the future. I wasn't sure if just leaving non-accessible code in with the production code is okay or not? Moreover, should tests relating to the reading/writing of datapackages be moved/deleted/commented out?
NOTE: As the issue mentioned, the frictionless datapackage should no longer be required. I haven't removed the package from requirements though, as the datapackage code still references it; therefore, linitng checks would flag this. After discussing question 2, hopefully, the answer to this comment will be clear :)