programminghistorian / ph-submissions

The repository and website hosting the peer review process for new Programming Historian lessons
http://programminghistorian.github.io/ph-submissions
135 stars 112 forks source link

Review Ticket: Getting Started with Word Embedding in R #145

Closed walshbr closed 6 years ago

walshbr commented 6 years ago

The Programming Historian has received the following tutorial on 'Getting Started with Word Embedding in R' by @SaraJKerr. The lesson is under review and can be found at:

http://programminghistorian.github.io/ph-submissions/lessons/getting-started-with-word-embeddings-in-r

Please feel free to use the line numbers provided on the preview if that helps with anchoring your comments, although you can structure your review as you see fit.

I will act as editor for the review process. My role is to solicit two reviews from the community and to manage the discussions, which should be held here on this forum. I have already read through the lesson and provided feedback, to which the author has responded.

Members of the wider community are also invited to offer constructive feedback which should post to this message thread, but they are asked to first read our Reviewer Guidelines (http://programminghistorian.org/reviewer-guidelines) and to adhere to our anti-harassment policy (below). We ask that all reviews stop after the second formal review has been submitted so that the author can focus on any revisions. I will make an announcement on this thread when that has occurred.

I will endeavor to keep the conversation open here on Github. If anyone feels the need to discuss anything privately, you are welcome to email me. You can always turn to @ianmilligan1 or @amandavisconti if you feel there's a need for an ombudsperson to step in.

Anti-Harassment Policy

This is a statement of the Programming Historian's principles and sets expectations for the tone and style of all correspondence between reviewers, authors, editors, and contributors to our public forums.

The Programming Historian is dedicated to providing an open scholarly environment that offers community participants the freedom to thoroughly scrutinize ideas, to ask questions, make suggestions, or to requests for clarification, but also provides a harassment-free space for all contributors to the project, regardless of gender, gender identity and expression, sexual orientation, disability, physical appearance, body size, race, age or religion, or technical experience. We do not tolerate harassment or ad hominem attacks of community participants in any form. Participants violating these rules may be expelled from the community at the discretion of the editorial board. If anyone witnesses or feels they have been the victim of the above described activity, please contact our ombudspeople (Ian Milligan and Amanda Visconti - http://programminghistorian.org/project-team). Thank you for helping us to create a safe space.

walshbr commented 6 years ago

@SaraJKerr looking at your markdown syntax issues now - sometimes when you link to something you have a space between the brackets and the parentheses, as in paragraph 3. Getting rid of the space between brackets and parentheses will make the links actually parse as links. And then the other main syntax issue I see is the footnotes - some of the footnotes look like they have extra line breaks in them. The parser expects any note to only exist on a single line, so a line break will make it confused and think it's moved onto a new note. If you can clean up those things I'll take a look at the content of the lesson itself! Let me know if you have any problems with this or with getting Jekyll running - happy to help out.

walshbr commented 6 years ago

Ah and I just added a folder for the lesson in the assets folder for you @SaraJKerr - you'll need to upload the Raw_pepys.txt file in there when you get a chance. I should be able to finish reading this and commenting by next week, though I've started already.

SaraJKerr commented 6 years ago

@walshbr I have uploaded the Pepys_raw.txt file. I have also set up Jekyll to run on my laptop but it throws an error with the images "Could not locate the included file 'figure.html' " so I think that is an issue with my local folder set up. I have corrected the markdown for the hyperlinks and a couple of the images where there were missing speech marks. Should I upload it directly or create a branch?

walshbr commented 6 years ago

@SaraJKerr - we don't employ a branching strategy on this staging repository so you can go ahead and upload directly.

I can also try to troubleshoot the include issue - it's not coming up when I run ph-submissions locally, so it does sound like an issue with your local setup. If you want to send along a screenshot of the error message in your terminal and your folder setup I can take a look. Happy to keep talking troubleshooting on the review ticket here or take things back to email if you'd like.

SaraJKerr commented 6 years ago

@walshbr - I have uploaded the corrected version, but it is still bringing up a 404 when I try to open it on GitHub pages - so there must be something I have missed.

I'll try to sort out the local set up, but thanks for the offer of help.

walshbr commented 6 years ago

It's possible you have the wrong link? The staging version is here (coming up for me) - http://programminghistorian.github.io/ph-submissions/lessons/getting-started-with-word-embeddings-in-r. If that's not coming up though it might be a caching issue - I'd try clearing your cache. But if that doesn't work let me know and can try to troubleshoot more.

I cleaned up the footnotes for you. I'll take a look in the next week.

SaraJKerr commented 6 years ago

Thank you - that works. I think I must have missed a letter somewhere.

walshbr commented 6 years ago

Had some time, so I took at this earlier than anticipated. I think this is great in a lot of ways and headed in the right direction! It will be a great addition to the text analysis and R offerings that we have. The recommendations I have below are almost entirely requests for more explanation, analysis, and narrativizing throughout the piece. In particular, I think more explanation of the concepts underlying the technique and more engagement with the Pepys material will help gear the piece towards a humanist audience and give a clearer sense of how people could use this in their own research. I've added all my running notes from the read through as checkboxes so that you can mark things off as you go. Let me know if you have any questions or want to discuss anything. I think it makes sense to make these revisions before sending out to reviewers, as what I'm suggesting below would add a fair amount of text.

SaraJKerr commented 6 years ago

Thank you for the feedback. I’m finishing a chapter at the moment but will get started on the list as soon as I’m done.

walshbr commented 6 years ago

Sounds great @SaraJKerr - let me know if you wanted to discuss anything!

walshbr commented 6 years ago

I've been communicating with @SaraJKerr - she's going to aim to get revisions back by the end of March. So I'll check back in around March 25th.

walshbr commented 6 years ago

@SaraJKerr - just checking in on this given that March 25th just passed. Do you have an update on revisions?

SaraJKerr commented 6 years ago

@walshbr - Hi Brandon, I have had a look at the suggestions you have made and in light of those, and further research, I want to do a major rewrite, and possibly change the focus text. I'm not going to have time to do it at the moment, but will have time in the Summer. Can we revisit it then?

walshbr commented 6 years ago

Yep totally! I'll close this for now. Why don't you send a new outline along when you're ready to my email, we can chat a little off github to help scope and frame, and then go from there? I'll make a note to ping you in July if I haven't heard from you. Will close for now, but we can reopen when you're ready.

walshbr commented 6 years ago

@SaraJKerr - my reminder went off to ping you again about this. Feel free to send something along for review when you have a chance. We can reopen the issue whenever you have a chance to direct attention towards this.