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

Fix #181:Introduce pretty print and pager output for challenges #197

Merged
merged 33 commits into from
Dec 15, 2019

Conversation

RohanSreelesh
Copy link
Contributor

The table that is displayed when the command evalai challenges is used has the respective colors:
1.ID-White
2.Title-Yellow
3.Short Description-Cyan
4.Creator-White
5.Start date- Green
6.End date -Red

It also has pager output enabled so that the command line output is not overwhelming

@pushkalkatara
Copy link

Hi @RohanSreelesh The build is failing.

Please add termcolor as a dependency and resolve flake8 formats.

requirements.txt Outdated
responses==0.9.0
validators==0.12.2
termcolor==1.1.0

Choose a reason for hiding this comment

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

Please remove this extra line.

@pushkalkatara
Copy link

To resolve flake8 issues:

  1. Locally install flake8 using pip python -m pip install flake8
  2. run the command - flake8 ~/dir/evalai-cli
  3. it would list some errors, resolve accordingly.

@@ -1,10 +1,10 @@
import json
import requests
import sys

from termcolor import colored
Copy link
Member

Choose a reason for hiding this comment

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

@RohanSreelesh please move this to last import after datetime. We follow alphabetical order for imports

from bs4 import BeautifulSoup
from beautifultable import BeautifulTable
from click import echo, style
from click import echo, style, echo_via_pager
Copy link
Member

Choose a reason for hiding this comment

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

move style after echo_via_pager

Copy link
Contributor Author

@RohanSreelesh RohanSreelesh Dec 9, 2019

Choose a reason for hiding this comment

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

ok I have moved it to the end and changed style and eco_via_pager

requirements.txt Outdated
coveralls==1.3.0
docker==3.6.0
flake8==3.0.4
lxml==4.2.1

Choose a reason for hiding this comment

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

@RohanSreelesh Why is lxml removed as a dependency in this commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My build wasnt installing , then @yashdusing on gitter suggested removing this from requirements and setup.py after which I was able to install it successfully
The error can be found here https://imgur.com/g4wefJP

Choose a reason for hiding this comment

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

Some development libraries are missing in the python package related to lxml, thus the package was not installing. You can find more details here.

We cannot remove it from the requirements as it would break some other parts of the code(for example, the build fails as the lmxl based tests are not passing).

The issue exists on windows due to the version of lxml (please refer to the second answer on the link about. maybe you could update the version of lxml in requirements.txt and try reinstalling on windows to resolve the issue.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand , I had done the solutions given in the link including the second answer but the build still didnt work for me unfortunately. I'll add it back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added lxml back into requirements and setup.py but the build is still failing @pushkalkatara

Choose a reason for hiding this comment

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

The build is failing due to some more issues. You can get all the details here.

  1. flake8 error
  2. the test outputs don't match (you need to update the tests also.)

@RohanSreelesh
Copy link
Contributor Author

removed echo_via_pager as it isnt giving consistent outputs

@RohanSreelesh
Copy link
Contributor Author

Update to tests based on the changes made in the above code

@pushkalkatara
Copy link

Good job @RohanSreelesh . Looks good to me. The output -
output

@yashdusing
Copy link

Also, @RohanSreelesh check for the case for the descriptions are too large or too small. I guess it would be okay to truncate the text after a certain limit. Also check for empty string

@pushkalkatara
Copy link

@yashdusing Another student is working on that task and there's an open PR for the same - #183

@RohanSreelesh
Copy link
Contributor Author

RohanSreelesh commented Dec 10, 2019

@pushkalkatara @yashdusing Is my work regarding this done? if so how may I proceed as my task on gci is still in submitted state. Thanks

@yashdusing
Copy link

@RohanSreelesh try synchronising the file by using the 2 files from an earlier build. Then, make changes to it

@RohanSreelesh
Copy link
Contributor Author

@RohanSreelesh try synchronising the file by using the 2 files from an earlier build. Then, make changes to it

Thanks @yashdusing that worked and now it shows only 1 line as newly added

@yashdusing
Copy link

@RohanSreelesh add the missing line as mentioned.

@RohanSreelesh
Copy link
Contributor Author

RohanSreelesh commented Dec 10, 2019

@RohanSreelesh add the missing line as mentioned.

which line @yashdusing ? I have added termcolor in both setup.py and requirements

@yashdusing
Copy link

@RohanSreelesh line 34 in setup.py. Please check

@yashdusing
Copy link

LGTM 👍
@Ram81 @pushkalkatara please verify

@pushkalkatara
Copy link

Looks good to me too. Good job @RohanSreelesh .

Copy link
Member

@RishabhJain2018 RishabhJain2018 left a comment

Choose a reason for hiding this comment

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

LGTM :)
Good work @RohanSreelesh 🎉

Thanks, @pushkalkatara @yashdusing @Ram81 for the quick reviews

@RishabhJain2018 RishabhJain2018 merged commit c744798 into Cloud-CV:master Dec 15, 2019
Ram81 pushed a commit to Ram81/evalai-cli that referenced this pull request Nov 9, 2020
…oud-CV#197)

* pretty print task

* Add files via upload

* Update requirements.txt

* Add files via upload

* Add files via upload

* Delete challenges.py

* Update challenges.py

* Update requirements.txt

* Update setup.py

* Update challenges.py

* Update challenges.py

* Update challenges.py

* Update challenges.py

* Update requirements.txt

* Update setup.py

* trying to add test

* fixed flask8 errors

* flake8 errors

* changed str(table) to table

* Update test_challenges.py

* Update test_challenges.py

* Update test_auth.py

* Update test_auth.py

* Update requirements.txt

* Add files via upload

* Update requirements.txt

* Update requirements.txt

* Add files via upload

* Add files via upload

* Update requirements.txt

* Add files via upload

* Update setup.py

* Update setup.py
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.

5 participants