New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Instructor trainning submission] Added new example to debugging episode with overlapping errors #340

Open
wants to merge 3 commits into
base: gh-pages
from

Conversation

Projects
None yet
4 participants
@panchyni

panchyni commented Jan 3, 2017

From the instructor training, I remember talking about editing R markdown files to contribute to lessons,
but I did not see any .rmd files in this repo and the CONTRIBUTING.md was not clear on this. As such, I made changed to the .md files. If this is an error, please let me know so that I can correct it. Thank you.

@tbekolay

This comment has been minimized.

Contributor

tbekolay commented Jan 4, 2017

Hi @panchyni thanks for your pull request! You're correct that this lesson doesn't have R markdown files, so we edit the .md files.

I like the idea behind this exercise, and it would be great to have more exercises in the debugging section! My main concern with how it is currently written is that it might be difficult for students who are not in genetics to understand what the correct solution should look like. For example, is it important to use the term "phenotype" when a more generic term would suffice? Why do you use A and B to refer to the two genes? What do the numbers in "phenotypes" mean, how should a student conceptualize these numbers? (I.e., what does it mean for Gene A to have phenotype value 0.5?)

Code-wise, it seems a little confusing to me that the count variable is passed into the function. Was this done to give something that's debuggable, or is this a common pattern that you've encountered? In these types of examples, it's a tough balancing act to write code that is buggy but still readable. It's all good if you've seen this often; in the code that I see more often, I would expect count to be created inside the function. Also, there are several style issue -- we try to adhere to PEP8 in code snippets, though we don't do any automated testing of this at the moment.

It would be great if you could address these concerns above as I think this could be a great exercise! Let me know if you want any help with rewordings or style.

@panchyni

This comment has been minimized.

panchyni commented Jan 4, 2017

Thanks for the feedback!

Sorry if there was too much jargon in the genetics example. I was actually trying to use as much generic language as possible, but based on your comments, I think this made things more confusing. Instead of "phenotype" and "deletion", I could reword the example to talk about how often an organism dies because of a mutation. The same principle would apply: if two mutations cause the organism to die more/less often than the sum, that would indicate interaction. Perhaps this would be a little clearer? Ultimately, the language is flexible, but I do like how the theme of gene interaction lines up with the lesson.

As to the use of count, it is something I have seen before, but it was a stylistic choice on my part. I have the habit of being (perhaps overly) explicit with my examples, because I expect students to forget things, like scope, in the courses I normally teach. This is one of many things I am going to have unlearn when writing for a more focused course. I will fix this and conform the code style to PEP8 for my next submission.

I will have an update for you (hopefully) soon.

panchyni added some commits Jan 5, 2017

panchyni
Rewrote AddInteraction function to remove the count argument and intr…
…oduced a new error by returning the same value for both if and else. Also rewrote the intro bit and cleaned up the formatting.
@panchyni

This comment has been minimized.

panchyni commented Jan 9, 2017

Hello @tbekolay,

I have rewritten my example based on your suggestions. I think the intro and code should be more readable now, but I am not as confident about the style. I went through the PEP8 specifications a couple times, but I'm still not that familiar with it, so I don't have a good sense of if I caught everything that needs to fixed.

maxim-belkin pushed a commit that referenced this pull request Aug 22, 2018

Merge pull request #340 from tracykteal/calendar
added workshop calendar

@maxim-belkin maxim-belkin force-pushed the swcarpentry:gh-pages branch from 4753644 to 90848ea Oct 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment