Cloud-CV / evalai-cli

:cloud: :rocket: Official EvalAI Command Line Tool
https://cli.eval.ai
BSD 3-Clause "New" or "Revised" License
55 stars 63 forks source link

Add command to display the stderr data from submission in terminal #252

Open hkmatsumoto opened 4 years ago

hkmatsumoto commented 4 years ago

The corresponding GCI task

Changes:

CC: @vkartik97 utsav @Ram81 @RishabhJain2018

hkmatsumoto commented 4 years ago

Since the content of stderr files are basically error messages, it is hard to distinguish stderr messages (i,e: intended behavior) from cli error messages (i,e: unexpected error) if we print the content directly like the figure below. A

To convince users that the output is a stderr message and cli is not crushing, I decided to print an extra message like the figure below. B

hkmatsumoto commented 4 years ago

And the content of stderr file is this: STDERR

hkmatsumoto commented 4 years ago

Hi @vkartik97, thanks for replying on the task page but I still can't find your review. Could you recomment it please?

krtkvrm commented 4 years ago

~Hi, Can you see this comment: https://github.com/Cloud-CV/EvalAI-ngx/pull/261#issuecomment-571625766?~

hkmatsumoto commented 4 years ago

Yes

krtkvrm commented 4 years ago

The comment is in pending status, so pasting here again: Why have you made a util: display_submission_stderr, which is only used by stderr, i.e stderr function in submissions.py. I am not able to determine making extracting out the not so large amount statements to a separate util function. Please let me know if I am missing something.

hkmatsumoto commented 4 years ago

@vkartik97 That is because I tried to make stderr command similar to other functions including submission and result command. submission calls display_submission_details and result calls display_submission_result.

hkmatsumoto commented 4 years ago

@vkartik97 I personally feel the same thing to you, so I am willing to make another PR that takes in the code of the functions in utils to the command function and wait for opinions from mentors including you.

krtkvrm commented 4 years ago

If you are going ahead for making PR as commented by you, can you only try for stderr(this PR) and result first?

hkmatsumoto commented 4 years ago

Sure but can I ask why?

krtkvrm commented 4 years ago

The refactoring will be less as compared to going for both result and submission. Once we are satisfied with the changes in result, you can go ahead. And pardon me, I didn't state the reason initially.

hkmatsumoto commented 4 years ago

@vkartik97 I got it. Created https://github.com/Cloud-CV/evalai-cli/pull/253.

hkmatsumoto commented 4 years ago

I fell good about the change made in #253. What do you think @vkartik97?

hkmatsumoto commented 4 years ago

@Ram81 @RishabhJain2018 Could you also take a look?

hkmatsumoto commented 4 years ago

I closed by accident

hkmatsumoto commented 4 years ago

Apart from #253, what do you think of the changes made in this PR?

krtkvrm commented 4 years ago

The PR for https://github.com/Cloud-CV/evalai-cli/pull/253 seems good, but is of independent of this task. So, approving this, as refactoring can be a new task altogether.