search

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 pretty format for command 'get_token' #195

Closed m-sameer closed 4 years ago

m-sameer commented 4 years ago

Fix #181: Add pretty format for command 'get_token'

Before: Screenshot from 2019-12-08 20-46-39 After: Screenshot from 2019-12-08 19-36-42

m-sameer commented 4 years ago

I would suggest that we build a colour palette with a string having a certain colour, a link having a certain colour, names of challenges, date, username etc.

What about that?

pushkalkatara commented 4 years ago

@m-sameer Yes, that's right. We all can have a discussion and decide a color.

m-sameer commented 4 years ago

Yes.

Links can be blue. The classics way. Token (and other strings) be green.

pushkalkatara commented 4 years ago

To manage the discussion we can list down commands which need to be designed. Blue links and green tokens would suite best.

Also, as @Ram81 added that we would need a more generalized manner and not hardcoded colors. To understand this context please have a look at challenge table design in which a single function is used to summarize all the challenge related CLI commands.

m-sameer commented 4 years ago

So what do I do?

Have some global variables (of colours) for different information?

m-sameer commented 4 years ago

@pushkalkatara How should I proceed further?

pushkalkatara commented 4 years ago

Yes, as in the challenge.py there is single function for all the challenge related commands. You can create for other files and commands as well. I have extended the time on GCI dashboard.

m-sameer commented 4 years ago

Where do I store the variables?

m-sameer commented 4 years ago

@pushkalkatara Please review.

pushkalkatara commented 4 years ago

@m-sameer Do we need a class to store variables?

m-sameer commented 4 years ago

We need a class to tell of what category the variables belongs to.

pushkalkatara commented 4 years ago

Can we think of other ways? Maybe a dictionary?

m-sameer commented 4 years ago

Why a dictionary though?

m-sameer commented 4 years ago

@pushkalkatara Please review.

pushkalkatara commented 4 years ago

Hi @m-sameer Sorry for the late response.

Here's the expectation from this issue:

  1. There are several commands here in https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/utils
  2. As you have chosen token related commands i.e. auth commands which are under the utils here - https://github.com/Cloud-CV/evalai-cli/blob/master/evalai/utils/auth.py
  3. You have to make changes to this file, write a function such as pretty_print_auth_commands or whatever name you like. This would generalize a function for all auth related commands.

Actually I just found that there is some replication in the code in evalai-cli/get_token.py and evalai-cli/utils/auth.py. I have created a separate issue to remove this replication #223 .

m-sameer commented 4 years ago

@pushkalkatara What do I do in that function?

Copy paste this function?

m-sameer commented 4 years ago

@pushkalkatara please reply

pushkalkatara commented 4 years ago

@m-sameer Create a function here named as pretty_print_auth_commands with necessary arguments just like here for challenges. And use this function in each auth related function in auth.py to pretty print on terminal.

I apologize for the late reply. I have been traveling recently.

pushkalkatara commented 4 years ago

For now, it looks good and we can approve the GCI task. I think we can more generalize the pretty print commands format by using it for other CLI commands too. One liner function doesn't look good, maybe when we extend it for other CLI commands, it would have a better format. What do you guys think @Ram81 @vkartik97

m-sameer commented 4 years ago

Well, I got screwed here. This PR wasn't scrutinised as much as mine was. Probably it's a GCI task PR.

m-sameer commented 4 years ago

@pushkalkatara Please reply.

pushkalkatara commented 4 years ago

@m-sameer Don't worry. This PR got a bit delayed, you did a good job but the expectations changed with time (like we discovered a new issue that there is code replication, also we discovered that we can more generalize the pretty-print function). I apologize if I was late with the response.

I am approving your GCI task as you have solved the issue and helped us to discover more issues. We'll keep the PR on hold for a bit until we resolve the replication of get_token command. As this PR and GCI task has several instances, and you are also familiar with the code, you can retake another pretty print functionality if you're interested.

m-sameer commented 4 years ago

@pushkalkatara Well I am having my feet cut off because of the extreme delay and that takes my 30 days from the contest.

I can never ever win this contest because of this single PR.

pushkalkatara commented 4 years ago

@m-sameer Hey don't worry you still have a lot of time and there are plenty of tasks. Keep your spirits up and make most learning out of GCI.