-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue223 #260
base: main
Are you sure you want to change the base?
Issue223 #260
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a couple of errors I encountered but looks good and an interesting approach to JSON encoding I wasn't aware of personally.
Apologies for the delay in getting to review this. If you're out of spare time now then please just let me know and I can look into investigating the errors if you're okay with that.
], | ||
"python.testing.unittestEnabled": false, | ||
"python.testing.pytestEnabled": true, | ||
"python.defaultInterpreterPath": "F:\\Program Files\\python3.9\\python" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would remove this as it's dependent on the machine config of whoever's running the scripts
|
||
processor.write_json(fname, prepped) | ||
|
||
processor = ProcessorUSMART() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
processor.process()
below needs updated to pass the file_type
param value of json
I think. When I ran this I was still getting .csv file names instead of .json files
@@ -110,7 +110,7 @@ def main(): | |||
source_usmart = pd.concat( | |||
[ | |||
source_usmart, | |||
pd.read_csv( | |||
pd.read_json( | |||
folder + r"/" + filename, | |||
parse_dates=["DateCreated", "DateUpdated"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line caused an error for me. I think it needs to be convert_dates
instead
Description
Previously, the usmart api script (usmart.py) outputs to a .csv which is then ingested into merge_data.py. This should be converted to a .json output to keep consistent with the rest of the project.
Changes done in a way which allows a mix and match approach while other providers are switched from csv to json
Motivation and Context
See original ticket
How Has This Been Tested?
Unit tests (pytest) written for JSON objects and testing reading/writing of the same
Screenshots (if appropriate):
Types of changes
Checklist: