Skip to content

Update names and date - #9

Merged
cmvcordova merged 4 commits into
QLS-MiCM:mainfrom
anna-zai1:patch-1
Oct 2, 2026
Merged

cmvcordova merged 4 commits into
QLS-MiCM:mainfrom
anna-zai1:patch-1

Conversation

@anna-zai1

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

PR Checklist

PR: #9
Author: @anna-zai1

Please confirm the following before requesting review:

  • You have read the workshop contribution guidelines
  • You have included a README outlining the workshop and its contents
  • You have included requirements and setup instructions in the README.md
  • You have tested the workshop exercises and data sets
  • You have organized your repo to match the workshop template structure
  • You have included a pdf copy of your slides to facilitate review
  • All changes or contributions are clearly explained in the PR description

Reply to this comment or check off the boxes when complete.

@cmvcordova cmvcordova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this updated ahead of tomorrow's session. There is one blocker in this PR. Since the notebook was due a proper look anyway, I also went through the student and solutions versions in full and ran every cell of the solutions notebook top to bottom. It executes without errors, but one result is wrong (2.1).

1. This PR

Blocker: the notebook is no longer valid JSON. The new instructor line is missing its trailing comma, so Jupyter, Colab and GitHub cannot open the file:

Expecting ',' delimiter: line 21 column 5

The inline suggestion on line 20 fixes it ("Commit suggestion" applies it in one click). It also adds the blank line so the name does not run into "Dear Reader…".

To settle before merging:

  • #8 edits the same lines (and has "Ocober"). Only one of the two can merge, so one should be closed.
  • The two PRs disagree on Cienna's affiliation: "MSc. Student, Human Genetics" here, "MSc Student, Quantitative Life Sciences" in #8.
  • The previous instructor and date are still shown elsewhere:
    • the solutions notebook header (Exercises/results/IntroToPython.ipynb): "February 18, 2026", Sameena Karsan;
    • the slides title page: "Lead: Sameena Karsan, June 3, 2026";
    • Outline/Outline.md: lead and facilitator. It also says "4-hour workshop" with 1 hour each for Modules 1 and 2, while the README says 2 hours and the notebook says 30 minutes each.
  • The first-person text now reads wrong. "I will introduce you…", "based on my previous workshop material", "my upcoming workshop Data Processing in Python" and "two of my previous workshops" (which links to @bzrudski's repos) are all Benjamin Rudski's voice. Suggested replacement for the intro line, matching the README: "This material was originally developed by Benjamin Rudski, with material from Najia Bouaddouch." The "my upcoming workshop" sentence in Module 5 should be updated or removed.

2. Code

2.1 translate ignores its argument (solutions, last exercise of Module 4). convert_mrna_to_codons(mrna) reads the global my_rna left over from Module 3 instead of its own parameter. The final test therefore prints MTESVSLRLRTGH, the Module 3 answer, whatever sequence is passed in. In a fresh kernel where the Module 3 cell has not been run it raises NameError: name 'my_rna' is not defined. With the parameter used, the test sequence gives MVS.

def convert_mrna_to_codons(mrna):
    my_codons = []

    start_codon_index = mrna.find("AUG")

    # No start codon: nothing to translate
    if start_codon_index == -1:
        return my_codons

    for i in range(start_codon_index, len(mrna) - 2, 3):
        my_codons.append(mrna[i: i + 3])

    return my_codons

The stored output of the test cell needs re-running after this (MVS). This is also a nice live example of why functions should use their parameters and not globals.

2.2 count_nucleotides counts anything unknown as G (both notebooks; it is pre-filled in the student version). The final else catches every character that is not A, T or C: count_nucleotides("ACGTNNNN") returns (1, 1, 1, 5). Suggested:

        elif nt == "C":
            number_of_c += 1
        elif nt == "G":
            number_of_g += 1

2.3 Smaller ones (solutions):

  • The text asks whether the product is "less than 28"; the solution tests < 30.
  • The while version of the Celsius table prints the header ===== FOR LOOP RESULTS ======.
  • The unit-conversion solution treats every unit other than "C" as Fahrenheit, right after the exercise that introduces "K".

3. Student notebook vs solutions

  • Three examples have no cell to type in. The text announces an example but the code cell exists only in the solutions:
    • after "Remember that the rules of BEMDAS apply" (the next cell then says "This example contained integers");
    • after "Let's now change the password to "World" and try again";
    • after "we can check if the nucleotide is contained in a string using the in keyword".
  • Dictionary iteration says "Your code here" but ships with the answer filled in.
  • Translation (Part II) says the codons "should still be in the variable my_codons", but the Part I scaffold never names that variable ("Create an empty codon list"). Adding my_codons = ... to the hint avoids a NameError for anyone who picked another name. It also says "from the DNA sequence"; it is the mRNA.
  • Solutions only: there is a stray copy of the image_counts average cell in the Module 3 exercises, and the Module 3 outline lists the Module 4 exercise title.

4. Teaching

What works well: one biological thread (GC content, transcription, codons, translation) runs through all four modules and ends with the same code wrapped into functions; the blanks with pre-written print lines keep live coding fast; and the "how to get help" module is worth keeping.

  • The diagrams are probably broken in Colab. The four images use relative paths (../assets/…). The README sends attendees to Colab through the GitHub link, where relative paths do not resolve. That affects both string-indexing diagrams, the function "machine" and the script progression, plus the ../results/ link to the solutions. Worth a quick check in Colab; absolute URLs fix it, for example:
    https://raw.githubusercontent.com/QLS-MiCM/IntroToPython/main/Exercises/assets/StringIndexingPositive.png
  • Pace. The module times add up to 140 minutes for a 2-hour slot, with about 90 code cells; Module 1 has 28 of them for 30 minutes. It helps to decide in advance what to skip. The parts already labelled Extra or BONUS are natural candidates (del, Celsius to Fahrenheit, for to while).
  • Order of concepts.
    • List slicing says "exactly the same way that we did with strings and tuples", but tuples come later.
    • Purines and pyrimidines are used in the string-iteration example before they are defined in the exercises.
  • Keyword arguments. The section equates "has a default value" with "keyword argument" and "no default" with "positional argument". Calling them "parameters with default values" avoids confusion once attendees see f(x=1) for a parameter without a default.
  • Type hints (seq: str) -> dict[str, int]) appear in the last exercise without ever being introduced. One sentence of explanation, or removing them, would do.
  • Module 5 says "I talked a bit about functions and classes" and "We've seen how to install and use packages". Neither classes nor packages are covered in this workshop.
  • Typos: "Less that or equal to", "let's a more complicated example", "intuitve", "We can use repeat code", "as a they happen", "has not additional features", "Tomas Beuszen" (Beuzen); README: "Juoyter".

Only section 1 needs to happen in this PR. Sections 2 to 4 can go in a follow-up, though 2.1 and the Colab images are worth doing before the workshop if there is time.

Comment thread Exercises/Script/IntroToPython.ipynb Outdated
cmvcordova added a commit that referenced this pull request Oct 1, 2026
Reverted in the next commit.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cmvcordova added a commit that referenced this pull request Oct 1, 2026
This reverts commit 033374d.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@anna-zai1

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review! I’ve committed the updates to the student and solutions notebooks and the outline, including the instructor names and date, author attribution, revised first-person wording, and workshop timing. I can also confirm that Cienna is an MSc student in Human Genetics, so the affiliation listed here is correct.

@cmvcordova cmvcordova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks for the quick turnaround. The notebook opens again, the attribution and wording read well, and thanks for confirming Cienna's affiliation. Merging now so it is in place for today's session.

One thing for a follow-up: the solutions notebook (Exercises/results/IntroToPython.ipynb) is not part of this PR, so its header still shows February 18, 2026 and Sameena Karsan. The remaining items from the review (the translate fix in the solutions, the Colab image paths, the per-module times in the outline) can go in that same follow-up.

Good luck with the workshop!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants