Milestone 2 Review

[mohamad-amin]

Good job! My concerns are:

Please include a requirements.txt file also, apart from mentioning your requirements in the Readme (at the top of your repository).

Have you checked this file: https://github.com/UBC-MDS/tech_salary_predictor_canada_us/blob/main/docs/report.ipynb
It doesn't show the figures.
Also, what's your final conclusion out of this report? Hint from the milestone: "There should be a written narrative in this document that introduces and justifies your question, introduces the data set, presents the findings/results, and interprets the findings/results in context of the question."

Seems like not all your figures are uploaded to the tech_salary_predictor_canada_us/results/ dir. Am I right?

Your scripts are nice and to me, have descent quality, congrats! Just that it's good to not have any commented code on Github, as it's a version control system and you can always recover your previous code.

[suuuuperNOVA]

Hi Mohamad,

Thank you for your valuable feedback. Sorry to make you confused. report.ipynb is a part of our project report source file. The final report needs to use jupyter-book to build and then, everything including plots shows up. There is an alternative way to read the report. After following the steps in Usage or just running make all in the terminal under the project root directory, the report will be rendered in \docs\_build\html\report.html.

Regarding the point of final conclusion, we added the conclusion section in our final report. Please check commit@5e7dfc

Actually, we've deployed our report as github-page: https://ubc-mds.github.io/tech_salary_predictor_canada_us/
Hope you could enjoy it and we will add the requirements.txt.

Many thanks.

[khalidcawl]

Hi @mohamad-amin Thanks a lot for the feedback.

Please include a requirements.txt file
We are giving the user two options, Docker or conda installation. The conda installation uses yaml file in the root of our project. And the dockerfile has the dependencies in the Dockerfile. Therefore, I don't think we need requirements.txt

We will add Conclusion as well as limitations section as per this issue: #68

Thank you for the valuable feedback.