JRavi2 / Chat-Analyzer

A python program to analyze your Social Chats
12 stars 8 forks source link

Changes to streamline-io #16

Closed anitmangal closed 2 years ago

anitmangal commented 2 years ago

Closes #5

JRavi2 commented 2 years ago

@anitmangal Thanks for the PR! This is a nice start! Right now the results are getting printed out without much formatting. Could you change that to tabulated data? You can refer to the already implemented percentage part as an example.

anitmangal commented 2 years ago

Yes I will do that. I think only the 'activity' path requires the tabulated data formatting or is there something else as well?

@anitmangal Thanks for the PR! This is a nice start! Right now the results are getting printed out without much formatting. Could you change that to tabulated data? You can refer to the already implemented percentage part as an example.

JRavi2 commented 2 years ago

Also, I'd recommend running flake8 . from the src directory. This will list any linting errors. Most linting errors can be fixed by autopep8 running autopep8 -i -r . from the src directory. Others would have to be fixed manually. Both flake8 and autopep8 can be installed using pip.

anitmangal commented 2 years ago

Follow up on #16 and #17 addressing #5

JRavi2 commented 2 years ago

This looks good! One final thing left. Since the output for some of the options has changed, the tests need to be updated. To do that you just need to run the program for the test files and put in the updated output in the expected_res in test.py, like here and verify that all the tests pass.

Also, no need to worry about the linting for the tests.py file.

anitmangal commented 2 years ago

Follow up on issue #5

anitmangal commented 2 years ago

Follow Up

JRavi2 commented 2 years ago

Can you capitalize the hours_list variable (to HOURS_LIST)? It's considered a good convention to keep the global constants capitalized.

anitmangal commented 2 years ago

Follow Up

JRavi2 commented 2 years ago

Looking all good and merging at last! Thank you for your patience! And, congrats on getting your first PR merged :tada: