UBC-MDS / fifa-potential

Supervised machine learning model to predict potential rating of players in FIFA 22
https://ubc-mds.github.io/fifa-potential/high-potential-fifa-prediction-report.html
Other
0 stars 0 forks source link

Peer review feedback #42

Closed jcairn02 closed 7 months ago

jcairn02 commented 7 months ago

Data analysis review checklist

Reviewer: jcairn02

Conflict of interest

Code of Conduct

General checks

Documentation

Code quality

Reproducibility

Analysis report

Estimated hours spent reviewing: 1.5 hours

Review Comments:

Amazing job overall. Had to look hard to find issues.

Minor things: I suggest moving the scripts to a /scripts folder instead of the /src folder. As is, the /src ends up having multiple functions. /scripts is a clearer name. That’s how the great Tiffany Timbers organized her example repo.

https://github.com/ttimbers/breast_cancer_predictor_py

The report organization could be improved. I recommend a structure of summary of results -> introduction -> data-set-info/parameters -> results/discussion (how the best model/hyper-parameters were found) -> conclusion/analysis -> further improvements

I recommend making the pre-analysis section only about the pre-known variables and not about conclusions that were discovered from the results/discussion part (IE the model chosen). Move that part to after the results/discussion and change the wording to feel more like an exploration into a conclusion.

Finally, it is not clear how to run the tests. There is a mention of it in the readme that links to the tests folder, but the test folder doesn’t have any readme or clear way to indicate how to perform the tests. It would have taken me too much work to verify them, without a more detailed tutorial. An example:

https://github.com/ttimbers/breast_cancer_predictor_py/blob/main/tests/README.md

Great work tho! All issues were relatively minor. Keep it up!!

Attribution

This was derived from the JOSE review checklist and the ROpenSci review checklist.