Skip to content
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

create to_json function #75

Closed
wants to merge 7 commits into from
Closed

create to_json function #75

wants to merge 7 commits into from

Conversation

NoorShamasneh
Copy link
Contributor

Fixes #33

add new function "to_json" inside new file "utilities.py"

@NoorShamasneh NoorShamasneh requested review from bradlo and larf311 March 21, 2022 12:34
@@ -0,0 +1,97 @@

Copy link

Choose a reason for hiding this comment

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

I believe this file needs the license as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added 👍

'''
for i in range(len(keys) - 1):
key = keys[i]
nextValue = {} if type(keys[i+1]) is str else []
Copy link
Collaborator

Choose a reason for hiding this comment

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

python does not use camelCase for identifiers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all variables now are snake_case 👍

@@ -0,0 +1,42 @@
# Copyright 2021 RelationalAI, Inc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrong year in copyright header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@NoorShamasneh NoorShamasneh requested review from bradlo and larf311 April 4, 2022 12:37
@salamehsameera
Copy link
Contributor

can we merge this?

@larf311
Copy link

larf311 commented Jul 6, 2022

No, I think we were going to close this as I believe @bradlo had something else in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add helper method for converting results to JSON
4 participants