Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

Issue #61: Support json stats. #3

Merged
merged 1 commit into from
Jan 18, 2020
Merged

Issue #61: Support json stats. #3

merged 1 commit into from
Jan 18, 2020

Conversation

hawkinsw
Copy link
Contributor

No description provided.

@hawkinsw hawkinsw self-assigned this Jan 15, 2020
Copy link
Contributor

@mcomella mcomella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

result: str = ""
for date in sorted(calculations.keys()):
result += date + ", " + calculations[date] + "\n"
return result

def calculate(dirname: str, tipe: Type, product: str, result_file: str):
def json_format_calculations(calculations: Mapping[str, str]) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: not worth changing now but this might be safer & more intuitive to do with the built-in json library (which I think can automatically convert python data types to valid JSON strings).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point. I will definitely do that on a subsequent pass. I honestly didn't expect @ecsmyth would have success with this output so I didn't anticipate using the code as it stands. But, it works and we are going with it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants