BBBecky0913 / MY472_Final_assignment_Option2

0 stars 0 forks source link

Peer Review #1

Open ZJDL-jw opened 5 months ago

ZJDL-jw commented 5 months ago

Although the author gave good interpretations of the figures to support the conclusions strongly, the final report can be improved in four aspects: First, it would be better to explain some terms in advance and clearly, especially the confusing "relationship between S&S actions and priorities" in 3.3.3. Second, the report of 2205 words should be, at most, the required 750 words. Third, the figures from 3.1.2 2) to 3.3 are not replicable for the reason discussed in the Code Review. Lastly, even though the figures are compelling, it would be better to use different colour series between groups to make them easier to distinguish, such as the ones in 3.1.2.

Speaking of the codes, although the author created functions to increase modularity, significant things could be improved. In legibility, the author can add more comments to the codes without them, clarify the confusing comments, and start a new line after %>%. Another thing is that there are Chinese comments in the codes, which are unfriendly to non-Chinese readers. In efficiency, employing a nested for loop is unnecessary if only one value can iterate over. As for replicability, the major problem is the figures cannot be plotted again from 3.1.2 2) to 3.3, and errors always occur when encountering readRDS(). The author should include the out-source data and environment needed in the public repo to prevent consistent loss of objects. In addition, it'd better not hardcode with rename() to change column names as it is impossible to rerun.

BBBecky0913 commented 5 months ago

Thank you for the comments! And I really appreciate your advice. There are a few more explanations could be made though.

① Some terms of "relationship between S&S actions and priorities" in 3.3.3 have been explained at the beginning of 3.32 Neighborhood Priority. ② I have uploaded the data according to your request, there seem to be no requirements in advance for uploading my data so I missed it. Besides, I use readRDS() as the method only to cache the data, so although I didn't upload the cached data, other parts of my code shouldn't be not replicable. These code for caching can be skipped when running without affecting the main function :) ③ I only missed removing one or two lines in Chinese for to-do things, sorry for that, hope it won't affect the understanding of my overall code by non-Chinese readers, and of course no offense is intended :)

Finally, thank you again for your advice!