Skip to content

Added JSON output feature for inheritance tools as per issue #752#838

Open
michael-ford wants to merge 3 commits intoarq5x:masterfrom
michael-ford:master
Open

Added JSON output feature for inheritance tools as per issue #752#838
michael-ford wants to merge 3 commits intoarq5x:masterfrom
michael-ford:master

Conversation

@michael-ford
Copy link
Copy Markdown

As part of a course project we needed to contribute to an open-source database project so we have attempted to address issue #752 for Gemini. We used the argument format from the query command.

Copy link
Copy Markdown
Collaborator

@brentp brentp left a comment

Choose a reason for hiding this comment

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

thanks, this looks good. I've made suggestions inline.

Comment thread gemini/gemini_main.py
dest='format',
default='default',
help='Format of output (JSON or default)'
)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If all of the inheritance tools have this option, can you put this into the add_inheritance_args function?

Comment thread gemini/gemini_main.py
default='default',
help='Format of output (JSON or default)'
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

... into add_inheritance_args and same with below.

Comment thread gemini/gim.py
if i == 0:
has_gts = [x[0] for x in gt_cols_types if x[0] in s] or False
print("\t".join(s.keys()))
if self.args.format is 'default': print("\t".join(s.keys()))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

don't use is for string equality. use ==. It will probably always work due to interning, but makes me nervous.

Comment thread gemini/gim.py
yield item

def run(self):
test = []
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

name this something more descriptive than test please

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and define res or a better-named variable below where it's used.

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.

3 participants